refactor(@ngtools/webpack): remove forwardSlashPath utility usage from Ivy plugin

A cached path normalization helper is now used in places that still require normalization.  Usage was also removed completely in locations that are not needed either because the path was later normalized or the normalization would provide no benefit.
This commit is contained in:
Charles Lyding 2020-11-24 09:45:55 -05:00 committed by Alan Agius
parent 4c46c191c7
commit ff9e245a51
5 changed files with 70 additions and 50 deletions

View File

@ -11,7 +11,7 @@ import * as path from 'path';
import * as ts from 'typescript'; import * as ts from 'typescript';
import { NgccProcessor } from '../ngcc_processor'; import { NgccProcessor } from '../ngcc_processor';
import { WebpackResourceLoader } from '../resource_loader'; import { WebpackResourceLoader } from '../resource_loader';
import { forwardSlashPath } from '../utils'; import { normalizePath } from './paths';
export function augmentHostWithResources( export function augmentHostWithResources(
host: ts.CompilerHost, host: ts.CompilerHost,
@ -21,7 +21,7 @@ export function augmentHostWithResources(
const resourceHost = host as CompilerHost; const resourceHost = host as CompilerHost;
resourceHost.readResource = function (fileName: string) { resourceHost.readResource = function (fileName: string) {
const filePath = forwardSlashPath(fileName); const filePath = normalizePath(fileName);
if ( if (
options.directTemplateLoading && options.directTemplateLoading &&
@ -41,7 +41,7 @@ export function augmentHostWithResources(
}; };
resourceHost.resourceNameToFileName = function (resourceName: string, containingFile: string) { resourceHost.resourceNameToFileName = function (resourceName: string, containingFile: string) {
return forwardSlashPath(path.join(path.dirname(containingFile), resourceName)); return path.join(path.dirname(containingFile), resourceName);
}; };
resourceHost.getModifiedResourceFiles = function () { resourceHost.getModifiedResourceFiles = function () {
@ -157,7 +157,7 @@ export function augmentHostWithReplacements(
const normalizedReplacements: Record<string, string> = {}; const normalizedReplacements: Record<string, string> = {};
for (const [key, value] of Object.entries(replacements)) { for (const [key, value] of Object.entries(replacements)) {
normalizedReplacements[forwardSlashPath(key)] = forwardSlashPath(value); normalizedReplacements[normalizePath(key)] = normalizePath(value);
} }
const tryReplace = (resolvedModule: ts.ResolvedModule | undefined) => { const tryReplace = (resolvedModule: ts.ResolvedModule | undefined) => {

View File

@ -0,0 +1,42 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import * as nodePath from 'path';
const normalizationCache = new Map<string, string>();
export function normalizePath(path: string): string {
let result = normalizationCache.get(path);
if (result === undefined) {
result = nodePath.win32.normalize(path).replace(/\\/g, nodePath.posix.sep);
normalizationCache.set(path, result);
}
return result;
}
const externalizationCache = new Map<string, string>();
function externalizeForWindows(path: string): string {
let result = externalizationCache.get(path);
if (result === undefined) {
result = nodePath.win32.normalize(path);
externalizationCache.set(path, result);
}
return result;
}
export const externalizePath = (() => {
if (process.platform !== 'win32') {
return (path: string) => path;
}
return externalizeForWindows;
})();

View File

@ -18,7 +18,6 @@ import {
import { NgccProcessor } from '../ngcc_processor'; import { NgccProcessor } from '../ngcc_processor';
import { TypeScriptPathsPlugin } from '../paths-plugin'; import { TypeScriptPathsPlugin } from '../paths-plugin';
import { WebpackResourceLoader } from '../resource_loader'; import { WebpackResourceLoader } from '../resource_loader';
import { forwardSlashPath } from '../utils';
import { addError, addWarning } from '../webpack-diagnostics'; import { addError, addWarning } from '../webpack-diagnostics';
import { isWebpackFiveOrHigher, mergeResolverMainFields } from '../webpack-version'; import { isWebpackFiveOrHigher, mergeResolverMainFields } from '../webpack-version';
import { DiagnosticsReporter, createDiagnosticsReporter } from './diagnostics'; import { DiagnosticsReporter, createDiagnosticsReporter } from './diagnostics';
@ -30,6 +29,7 @@ import {
augmentHostWithSubstitutions, augmentHostWithSubstitutions,
augmentProgramWithVersioning, augmentProgramWithVersioning,
} from './host'; } from './host';
import { externalizePath, normalizePath } from './paths';
import { AngularPluginSymbol, FileEmitter } from './symbol'; import { AngularPluginSymbol, FileEmitter } from './symbol';
import { createWebpackSystem } from './system'; import { createWebpackSystem } from './system';
import { createAotTransformers, createJitTransformers, mergeTransformers } from './transformation'; import { createAotTransformers, createJitTransformers, mergeTransformers } from './transformation';
@ -180,7 +180,7 @@ export class AngularWebpackPlugin {
// Create a Webpack-based TypeScript compiler host // Create a Webpack-based TypeScript compiler host
const system = createWebpackSystem( const system = createWebpackSystem(
compiler.inputFileSystem, compiler.inputFileSystem,
forwardSlashPath(compiler.context), normalizePath(compiler.context),
); );
const host = ts.createIncrementalCompilerHost(compilerOptions, system); const host = ts.createIncrementalCompilerHost(compilerOptions, system);
@ -190,7 +190,8 @@ export class AngularWebpackPlugin {
// Invalidate existing cache based on compilation file timestamps // Invalidate existing cache based on compilation file timestamps
for (const [file, time] of compilation.fileTimestamps) { for (const [file, time] of compilation.fileTimestamps) {
if (this.buildTimestamp < time) { if (this.buildTimestamp < time) {
cache.delete(forwardSlashPath(file)); // Cache stores paths using the POSIX directory separator
cache.delete(normalizePath(file));
} }
} }
} else { } else {
@ -254,7 +255,7 @@ export class AngularWebpackPlugin {
const rebuild = (filename: string) => new Promise<void>((resolve) => { const rebuild = (filename: string) => new Promise<void>((resolve) => {
const module = modules.find( const module = modules.find(
({ resource }: compilation.Module & { resource?: string }) => ({ resource }: compilation.Module & { resource?: string }) =>
resource && forwardSlashPath(resource) === filename, resource && normalizePath(resource) === filename,
); );
if (!module) { if (!module) {
resolve(); resolve();
@ -279,7 +280,7 @@ export class AngularWebpackPlugin {
.map((sourceFile) => sourceFile.fileName), .map((sourceFile) => sourceFile.fileName),
); );
modules.forEach(({ resource }: compilation.Module & { resource?: string }) => { modules.forEach(({ resource }: compilation.Module & { resource?: string }) => {
const sourceFile = resource && builder.getSourceFile(forwardSlashPath(resource)); const sourceFile = resource && builder.getSourceFile(resource);
if (!sourceFile) { if (!sourceFile) {
return; return;
} }
@ -408,8 +409,7 @@ export class AngularWebpackPlugin {
const getDependencies = (sourceFile: ts.SourceFile) => { const getDependencies = (sourceFile: ts.SourceFile) => {
const dependencies = []; const dependencies = [];
for (const resourceDependency of angularCompiler.getResourceDependencies(sourceFile)) { for (const resourcePath of angularCompiler.getResourceDependencies(sourceFile)) {
const resourcePath = forwardSlashPath(resourceDependency);
dependencies.push( dependencies.push(
resourcePath, resourcePath,
// Retrieve all dependencies of the resource (stylesheet imports, etc.) // Retrieve all dependencies of the resource (stylesheet imports, etc.)
@ -444,8 +444,7 @@ export class AngularWebpackPlugin {
// NOTE: This can be removed once support for the deprecated lazy route string format is removed // NOTE: This can be removed once support for the deprecated lazy route string format is removed
for (const lazyRoute of angularCompiler.listLazyRoutes()) { for (const lazyRoute of angularCompiler.listLazyRoutes()) {
const [routeKey] = lazyRoute.route.split('#'); const [routeKey] = lazyRoute.route.split('#');
const routePath = forwardSlashPath(lazyRoute.referencedModule.filePath); this.lazyRouteMap[routeKey] = lazyRoute.referencedModule.filePath;
this.lazyRouteMap[routeKey] = routePath;
} }
return this.createFileEmitter( return this.createFileEmitter(
@ -513,8 +512,7 @@ export class AngularWebpackPlugin {
const pendingAnalysis = angularCompiler.analyzeAsync().then(() => { const pendingAnalysis = angularCompiler.analyzeAsync().then(() => {
for (const lazyRoute of angularCompiler.listLazyRoutes()) { for (const lazyRoute of angularCompiler.listLazyRoutes()) {
const [routeKey] = lazyRoute.route.split('#'); const [routeKey] = lazyRoute.route.split('#');
const routePath = forwardSlashPath(lazyRoute.referencedModule.filePath); this.lazyRouteMap[routeKey] = lazyRoute.referencedModule.filePath;
this.lazyRouteMap[routeKey] = routePath;
} }
return this.createFileEmitter(builder, transformers, () => []); return this.createFileEmitter(builder, transformers, () => []);

View File

@ -7,45 +7,25 @@
*/ */
import * as ts from 'typescript'; import * as ts from 'typescript';
import { InputFileSystem } from 'webpack'; import { InputFileSystem } from 'webpack';
import { externalizePath } from './paths';
function shouldNotWrite(): never { function shouldNotWrite(): never {
throw new Error('Webpack TypeScript System should not write.'); throw new Error('Webpack TypeScript System should not write.');
} }
// Webpack's CachedInputFileSystem uses the default directory separator in the paths it uses
// for keys to its cache. If the keys do not match then the file watcher will not purge outdated
// files and cause stale data to be used in the next rebuild. TypeScript always uses a `/` (POSIX)
// directory separator internally which is also supported with Windows system APIs. However,
// if file operations are performed with the non-default directory separator, the Webpack cache
// will contain a key that will not be purged.
function createToSystemPath(): (path: string) => string {
if (process.platform === 'win32') {
const cache = new Map<string, string>();
return (path) => {
let value = cache.get(path);
if (value === undefined) {
value = path.replace(/\//g, '\\');
cache.set(path, value);
}
return value;
};
}
// POSIX-like platforms retain the existing directory separator
return (path) => path;
}
export function createWebpackSystem(input: InputFileSystem, currentDirectory: string): ts.System { export function createWebpackSystem(input: InputFileSystem, currentDirectory: string): ts.System {
const toSystemPath = createToSystemPath(); // Webpack's CachedInputFileSystem uses the default directory separator in the paths it uses
// for keys to its cache. If the keys do not match then the file watcher will not purge outdated
// files and cause stale data to be used in the next rebuild. TypeScript always uses a `/` (POSIX)
// directory separator internally which is also supported with Windows system APIs. However,
// if file operations are performed with the non-default directory separator, the Webpack cache
// will contain a key that will not be purged. `externalizePath` ensures the paths are as expected.
const system: ts.System = { const system: ts.System = {
...ts.sys, ...ts.sys,
readFile(path: string) { readFile(path: string) {
let data; let data;
try { try {
data = input.readFileSync(toSystemPath(path)); data = input.readFileSync(externalizePath(path));
} catch { } catch {
return undefined; return undefined;
} }
@ -60,28 +40,28 @@ export function createWebpackSystem(input: InputFileSystem, currentDirectory: st
}, },
getFileSize(path: string) { getFileSize(path: string) {
try { try {
return input.statSync(toSystemPath(path)).size; return input.statSync(externalizePath(path)).size;
} catch { } catch {
return 0; return 0;
} }
}, },
fileExists(path: string) { fileExists(path: string) {
try { try {
return input.statSync(toSystemPath(path)).isFile(); return input.statSync(externalizePath(path)).isFile();
} catch { } catch {
return false; return false;
} }
}, },
directoryExists(path: string) { directoryExists(path: string) {
try { try {
return input.statSync(toSystemPath(path)).isDirectory(); return input.statSync(externalizePath(path)).isDirectory();
} catch { } catch {
return false; return false;
} }
}, },
getModifiedTime(path: string) { getModifiedTime(path: string) {
try { try {
return input.statSync(toSystemPath(path)).mtime; return input.statSync(externalizePath(path)).mtime;
} catch { } catch {
return undefined; return undefined;
} }

View File

@ -11,7 +11,7 @@
import * as path from 'path'; import * as path from 'path';
import * as vm from 'vm'; import * as vm from 'vm';
import { RawSource } from 'webpack-sources'; import { RawSource } from 'webpack-sources';
import { forwardSlashPath } from './utils'; import { normalizePath } from './ivy/paths';
const NodeTemplatePlugin = require('webpack/lib/node/NodeTemplatePlugin'); const NodeTemplatePlugin = require('webpack/lib/node/NodeTemplatePlugin');
const NodeTargetPlugin = require('webpack/lib/node/NodeTargetPlugin'); const NodeTargetPlugin = require('webpack/lib/node/NodeTargetPlugin');
@ -44,7 +44,7 @@ export class WebpackResourceLoader {
this.changedFiles.clear(); this.changedFiles.clear();
for (const [file, time] of parentCompilation.fileTimestamps) { for (const [file, time] of parentCompilation.fileTimestamps) {
if (this.buildTimestamp < time) { if (this.buildTimestamp < time) {
this.changedFiles.add(forwardSlashPath(file)); this.changedFiles.add(normalizePath(file));
} }
} }
} }
@ -159,7 +159,7 @@ export class WebpackResourceLoader {
// Save the dependencies for this resource. // Save the dependencies for this resource.
this._fileDependencies.set(filePath, new Set(childCompilation.fileDependencies)); this._fileDependencies.set(filePath, new Set(childCompilation.fileDependencies));
for (const file of childCompilation.fileDependencies) { for (const file of childCompilation.fileDependencies) {
const resolvedFile = forwardSlashPath(file); const resolvedFile = normalizePath(file);
const entry = this._reverseDependencies.get(resolvedFile); const entry = this._reverseDependencies.get(resolvedFile);
if (entry) { if (entry) {
entry.add(filePath); entry.add(filePath);