Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(@angular-devkit/build-angular): don't use parent modules while deduping #18438

Merged
merged 1 commit into from Aug 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -488,7 +488,6 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
modules: [wco.tsConfig.options.baseUrl || projectRoot, 'node_modules'],
plugins: [
PnpWebpackPlugin,
new DedupeModuleResolvePlugin({ verbose: buildOptions.verbose }),
],
},
resolveLoader: {
Expand Down Expand Up @@ -606,6 +605,7 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
// https://github.com/angular/angular/issues/11580
// With VE the correct context is added in @ngtools/webpack, but Ivy doesn't need it at all.
new ContextReplacementPlugin(/\@angular(\\|\/)core(\\|\/)/),
new DedupeModuleResolvePlugin({ verbose: buildOptions.verbose }),
...extraPlugins,
],
};
Expand Down
Expand Up @@ -6,89 +6,87 @@
* found in the LICENSE file at https://angular.io/license
*/

interface NormalModuleFactoryRequest {
import { Compiler } from 'webpack';

interface AfterResolveResult {
request: string;
resource: string;
context: {
issuer: string;
};
relativePath: string;
path: string;
descriptionFileData: {
name?: string;
version?: string;
resourceResolveData: {
relativePath: string;
descriptionFileRoot: string;
descriptionFilePath: string;
descriptionFileData: {
name?: string;
version?: string;
};
};
descriptionFileRoot: string;
descriptionFilePath: string;
directory?: boolean;
file?: boolean;
}

export interface DedupeModuleResolvePluginOptions {
verbose?: boolean;
}

/**
* DedupeModuleResolvePlugin is a webpack resolver plugin which dedupes modules with the same name and versions
* DedupeModuleResolvePlugin is a webpack plugin which dedupes modules with the same name and versions
* that are laid out in different parts of the node_modules tree.
*
* This is needed because Webpack relies on package managers to hoist modules and doesn't have any deduping logic.
*
* This is similar to how Webpack's 'NormalModuleReplacementPlugin' works
* @see https://github.com/webpack/webpack/blob/4a1f068828c2ab47537d8be30d542cd3a1076db4/lib/NormalModuleReplacementPlugin.js#L9
*/
export class DedupeModuleResolvePlugin {
modules = new Map<string, NormalModuleFactoryRequest>();
modules = new Map<string, { request: string, resource: string }>();

constructor(private options?: DedupeModuleResolvePluginOptions) { }

// tslint:disable-next-line: no-any
apply(resolver: any) {
resolver
.getHook('before-described-relative')
.tapPromise('DedupeModuleResolvePlugin', async (request: NormalModuleFactoryRequest) => {
if (request.relativePath !== '.') {
apply(compiler: Compiler) {
compiler.hooks.normalModuleFactory.tap('DedupeModuleResolvePlugin', nmf => {
nmf.hooks.afterResolve.tap('DedupeModuleResolvePlugin', (result: AfterResolveResult | undefined) => {
if (!result) {
return;
}

// When either of these properties is undefined. It typically means it's a link.
// In which case we shouldn't try to dedupe it.
if (request.file === undefined || request.directory === undefined) {
return;
}
const { resource, request, resourceResolveData } = result;
const { descriptionFileData, relativePath } = resourceResolveData;

// Empty name or versions are no valid primary entrypoints of a library
if (!request.descriptionFileData.name || !request.descriptionFileData.version) {
// Empty name or versions are no valid primary entrypoints of a library
if (!descriptionFileData.name || !descriptionFileData.version) {
return;
}

const moduleId = request.descriptionFileData.name + '@' + request.descriptionFileData.version;
const moduleId = descriptionFileData.name + '@' + descriptionFileData.version + ':' + relativePath;
const prevResolvedModule = this.modules.get(moduleId);

if (!prevResolvedModule) {
// This is the first time we visit this module.
this.modules.set(moduleId, request);
this.modules.set(moduleId, {
resource,
request,
});

return;
}

const {
path,
descriptionFilePath,
descriptionFileRoot,
} = prevResolvedModule;

if (request.path === path) {
const { resource: prevResource, request: prevRequest } = prevResolvedModule;
if (result.resource === prevResource) {
// No deduping needed.
// Current path and previously resolved path are the same.
return;
}

if (this.options?.verbose) {
// tslint:disable-next-line: no-console
console.warn(`[DedupeModuleResolvePlugin]: ${request.path} -> ${path}`);
console.warn(`[DedupeModuleResolvePlugin]: ${result.resource} -> ${prevResource}`);
}

// Alter current request with previously resolved module.
request.path = path;
request.descriptionFileRoot = descriptionFileRoot;
request.descriptionFilePath = descriptionFilePath;
result.request = prevRequest;
});
});
}
}
4 changes: 2 additions & 2 deletions tests/legacy-cli/e2e/tests/misc/dedupe-duplicate-modules.ts
Expand Up @@ -32,10 +32,10 @@ export default async function () {
const { stderr } = await ng('build', '--verbose', '--no-vendor-chunk', '--no-progress');
const outFile = 'dist/test-project/main.js';

if (/\[DedupeModuleResolvePlugin\]:.+tslib-1-copy -> .+tslib-1/.test(stderr)) {
if (/\[DedupeModuleResolvePlugin\]:.+tslib-1-copy\/.+ -> .+tslib-1\/.+/.test(stderr)) {
await expectFileToMatch(outFile, './node_modules/tslib-1/tslib.es6.js');
await expectToFail(() => expectFileToMatch(outFile, './node_modules/tslib-1-copy/tslib.es6.js'));
} else if (/\[DedupeModuleResolvePlugin\]:.+tslib-1 -> .+tslib-1-copy/.test(stderr)) {
} else if (/\[DedupeModuleResolvePlugin\]:.+tslib-1\/.+ -> .+tslib-1-copy\/.+/.test(stderr)) {
await expectFileToMatch(outFile, './node_modules/tslib-1-copy/tslib.es6.js');
await expectToFail(() => expectFileToMatch(outFile, './node_modules/tslib-1/tslib.es6.js'));
} else {
Expand Down