From f2cffa79cb298977a8925b30f8709f95e558e4ac Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Tue, 17 Jul 2018 20:06:39 -0400 Subject: [PATCH] refactor(@ngtools/webpack): use webpack resolver plugin for path mapping --- packages/ngtools/webpack/package.json | 1 + .../webpack/src/angular_compiler_plugin.ts | 25 +-- packages/ngtools/webpack/src/paths-plugin.ts | 193 +++++++++--------- packages/ngtools/webpack/src/webpack.ts | 2 + .../e2e/tests/misc/module-resolution.ts | 4 +- yarn.lock | 2 +- 6 files changed, 112 insertions(+), 115 deletions(-) diff --git a/packages/ngtools/webpack/package.json b/packages/ngtools/webpack/package.json index 2d0ddfed3aec..0acb8d5cfebf 100644 --- a/packages/ngtools/webpack/package.json +++ b/packages/ngtools/webpack/package.json @@ -22,6 +22,7 @@ "homepage": "https://github.com/angular/angular-cli/tree/master/packages/@ngtools/webpack", "dependencies": { "@angular-devkit/core": "0.0.0", + "enhanced-resolve": "4.1.0", "rxjs": "6.2.2", "tree-kill": "1.2.0", "webpack-sources": "1.2.0" diff --git a/packages/ngtools/webpack/src/angular_compiler_plugin.ts b/packages/ngtools/webpack/src/angular_compiler_plugin.ts index 9523125f88b7..410b9da63130 100644 --- a/packages/ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/ngtools/webpack/src/angular_compiler_plugin.ts @@ -32,7 +32,7 @@ import { WebpackCompilerHost, workaroundResolve } from './compiler_host'; import { resolveEntryModuleFromMain } from './entry_resolver'; import { gatherDiagnostics, hasErrors } from './gather_diagnostics'; import { LazyRouteMap, findLazyRoutes } from './lazy_routes'; -import { resolveWithPaths } from './paths-plugin'; +import { TypeScriptPathsPlugin } from './paths-plugin'; import { WebpackResourceLoader } from './resource_loader'; import { exportLazyModuleMap, @@ -729,6 +729,14 @@ export class AngularCompilerPlugin { }); compiler.hooks.afterResolvers.tap('angular-compiler', compiler => { + // tslint:disable-next-line:no-any + (compiler as any).resolverFactory.hooks.resolver + .for('normal') + // tslint:disable-next-line:no-any + .tap('angular-compiler', (resolver: any) => { + new TypeScriptPathsPlugin(this._compilerOptions).apply(resolver); + }); + compiler.hooks.normalModuleFactory.tap('angular-compiler', nmf => { // Virtual file system. // TODO: consider if it's better to remove this plugin and instead make it wait on the @@ -754,21 +762,6 @@ export class AngularCompilerPlugin { ); }); }); - - compiler.hooks.normalModuleFactory.tap('angular-compiler', nmf => { - nmf.hooks.beforeResolve.tapAsync( - 'angular-compiler', - (request: NormalModuleFactoryRequest, callback: Callback) => { - resolveWithPaths( - request, - callback, - this._compilerOptions, - this._compilerHost, - this._moduleResolutionCache, - ); - }, - ); - }); } private async _make(compilation: compilation.Compilation) { diff --git a/packages/ngtools/webpack/src/paths-plugin.ts b/packages/ngtools/webpack/src/paths-plugin.ts index 591ed0c9f492..cbefe70e39d4 100644 --- a/packages/ngtools/webpack/src/paths-plugin.ts +++ b/packages/ngtools/webpack/src/paths-plugin.ts @@ -6,54 +6,97 @@ * found in the LICENSE file at https://angular.io/license */ import * as path from 'path'; -import * as ts from 'typescript'; -import { - Callback, - NormalModuleFactoryRequest, -} from './webpack'; - - -export function resolveWithPaths( - request: NormalModuleFactoryRequest, - callback: Callback, - compilerOptions: ts.CompilerOptions, - host: ts.CompilerHost, - cache?: ts.ModuleResolutionCache, -) { - if (!request || !request.request || !compilerOptions.paths) { - callback(null, request); - - return; - } +import { CompilerOptions, MapLike } from 'typescript'; +import { NormalModuleFactoryRequest } from './webpack'; - // Only work on Javascript/TypeScript issuers. - if (!request.contextInfo.issuer || !request.contextInfo.issuer.match(/\.[jt]sx?$/)) { - callback(null, request); +const getInnerRequest = require('enhanced-resolve/lib/getInnerRequest'); - return; - } +export interface TypeScriptPathsPluginOptions extends Pick { - const originalRequest = request.request.trim(); +} - // Relative requests are not mapped - if (originalRequest.startsWith('.') || originalRequest.startsWith('/')) { - callback(null, request); +export class TypeScriptPathsPlugin { + constructor(private _options: TypeScriptPathsPluginOptions) { } - return; - } - - // Amd requests are not mapped - if (originalRequest.startsWith('!!webpack amd')) { - callback(null, request); + // tslint:disable-next-line:no-any + apply(resolver: any) { + if (!this._options.paths || Object.keys(this._options.paths).length === 0) { + return; + } - return; + const target = resolver.ensureHook('resolve'); + const resolveAsync = (request: NormalModuleFactoryRequest, requestContext: {}) => { + return new Promise((resolve, reject) => { + resolver.doResolve( + target, + request, + '', + requestContext, + (error: Error | null, result: NormalModuleFactoryRequest | undefined) => { + if (error) { + reject(error); + } else { + resolve(result); + } + }, + ); + }); + }; + + resolver.getHook('described-resolve').tapPromise( + 'TypeScriptPathsPlugin', + async (request: NormalModuleFactoryRequest, resolveContext: {}) => { + if (!request || request.typescriptPathMapped) { + return; + } + + const originalRequest = getInnerRequest(resolver, request); + if (!originalRequest) { + return; + } + + // Only work on Javascript/TypeScript issuers. + if (!request.context.issuer || !request.context.issuer.match(/\.[jt]sx?$/)) { + return; + } + + // Relative or absolute requests are not mapped + if (originalRequest.startsWith('.') || originalRequest.startsWith('/')) { + return; + } + + // Amd requests are not mapped + if (originalRequest.startsWith('!!webpack amd')) { + return; + } + + const replacements = findReplacements(originalRequest, this._options.paths || {}); + for (const potential of replacements) { + const potentialRequest = { + ...request, + request: path.resolve(this._options.baseUrl || '', potential), + typescriptPathMapped: true, + }; + const result = await resolveAsync(potentialRequest, resolveContext); + + if (result) { + return result; + } + } + }, + ); } +} +function findReplacements( + originalRequest: string, + paths: MapLike, +): Iterable { // check if any path mapping rules are relevant const pathMapOptions = []; - for (const pattern in compilerOptions.paths) { + for (const pattern in paths) { // get potentials and remove duplicates; JS Set maintains insertion order - const potentials = Array.from(new Set(compilerOptions.paths[pattern])); + const potentials = Array.from(new Set(paths[pattern])); if (potentials.length === 0) { // no potential replacements so skip continue; @@ -96,9 +139,7 @@ export function resolveWithPaths( } if (pathMapOptions.length === 0) { - callback(null, request); - - return; + return []; } // exact matches take priority then largest prefix match @@ -112,63 +153,23 @@ export function resolveWithPaths( } }); - if (pathMapOptions[0].potentials.length === 1) { - const onlyPotential = pathMapOptions[0].potentials[0]; - let replacement; - const starIndex = onlyPotential.indexOf('*'); - if (starIndex === -1) { - replacement = onlyPotential; - } else if (starIndex === onlyPotential.length - 1) { - replacement = onlyPotential.slice(0, -1) + pathMapOptions[0].partial; - } else { - const [prefix, suffix] = onlyPotential.split('*'); - replacement = prefix + pathMapOptions[0].partial + suffix; - } - - request.request = path.resolve(compilerOptions.baseUrl || '', replacement); - callback(null, request); - - return; - } - - // TODO: The following is used when there is more than one potential and will not be - // needed once this is turned into a full webpack resolver plugin - - const moduleResolver = ts.resolveModuleName( - originalRequest, - request.contextInfo.issuer, - compilerOptions, - host, - cache, - ); - - const moduleFilePath = moduleResolver.resolvedModule - && moduleResolver.resolvedModule.resolvedFileName; - - // If there is no result, let webpack try to resolve - if (!moduleFilePath) { - callback(null, request); - - return; - } - - // If TypeScript gives us a `.d.ts`, it is probably a node module - if (moduleFilePath.endsWith('.d.ts')) { - // If in a package, let webpack resolve the package - const packageRootPath = path.join(path.dirname(moduleFilePath), 'package.json'); - if (!host.fileExists(packageRootPath)) { - // Otherwise, if there is a file with a .js extension use that - const jsFilePath = moduleFilePath.slice(0, -5) + '.js'; - if (host.fileExists(jsFilePath)) { - request.request = jsFilePath; + const replacements: string[] = []; + pathMapOptions.forEach(option => { + for (const potential of option.potentials) { + let replacement; + const starIndex = potential.indexOf('*'); + if (starIndex === -1) { + replacement = potential; + } else if (starIndex === potential.length - 1) { + replacement = potential.slice(0, -1) + option.partial; + } else { + const [prefix, suffix] = potential.split('*'); + replacement = prefix + option.partial + suffix; } - } - - callback(null, request); - return; - } + replacements.push(replacement); + } + }); - request.request = moduleFilePath; - callback(null, request); + return replacements; } diff --git a/packages/ngtools/webpack/src/webpack.ts b/packages/ngtools/webpack/src/webpack.ts index e9035fb7b9e6..83d2a5cddf31 100644 --- a/packages/ngtools/webpack/src/webpack.ts +++ b/packages/ngtools/webpack/src/webpack.ts @@ -16,7 +16,9 @@ export interface Callback { export interface NormalModuleFactoryRequest { request: string; + context: { issuer: string }; contextInfo: { issuer: string }; + typescriptPathMapped?: boolean; } export interface InputFileSystem { diff --git a/tests/legacy-cli/e2e/tests/misc/module-resolution.ts b/tests/legacy-cli/e2e/tests/misc/module-resolution.ts index f54e64c7b55f..0396650258c7 100644 --- a/tests/legacy-cli/e2e/tests/misc/module-resolution.ts +++ b/tests/legacy-cli/e2e/tests/misc/module-resolution.ts @@ -73,14 +73,14 @@ export default async function () { await updateJsonFile('tsconfig.json', tsconfig => { tsconfig.compilerOptions.paths = { - '@firebase/polyfill': ['@firebase/polyfill/index.ts'], + '@firebase/polyfill': ['./node_modules/@firebase/polyfill/index.ts'], }; }); await expectToFail(() => ng('build')); await updateJsonFile('tsconfig.json', tsconfig => { tsconfig.compilerOptions.paths = { - '@firebase/polyfill*': ['@firebase/polyfill/index.ts'], + '@firebase/polyfill*': ['./node_modules/@firebase/polyfill/index.ts'], }; }); await expectToFail(() => ng('build')); diff --git a/yarn.lock b/yarn.lock index 30fa0b8d6592..65935810251f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2555,7 +2555,7 @@ engine.io@~3.2.0: engine.io-parser "~2.1.0" ws "~3.3.1" -enhanced-resolve@^4.1.0: +enhanced-resolve@4.1.0, enhanced-resolve@^4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/enhanced-resolve/-/enhanced-resolve-4.1.0.tgz#41c7e0bfdfe74ac1ffe1e57ad6a5c6c9f3742a7f" dependencies: