Skip to content

Commit eca08cc

Browse files
alan-agius4clydin
authored andcommitted
fix(@angular-devkit/build-angular): don't use parent modules while deduping
With this change we change the `DedupeModuleResolvePlugin` to act similar to `NormalModuleReplacementPlugin` Closes: #18411
1 parent 875e8b7 commit eca08cc

File tree

3 files changed

+38
-40
lines changed

3 files changed

+38
-40
lines changed

packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/common.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,6 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
476476
modules: [wco.tsConfig.options.baseUrl || projectRoot, 'node_modules'],
477477
plugins: [
478478
PnpWebpackPlugin,
479-
new DedupeModuleResolvePlugin({ verbose: buildOptions.verbose }),
480479
],
481480
},
482481
resolveLoader: {
@@ -594,6 +593,7 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
594593
// https://github.com/angular/angular/issues/11580
595594
// With VE the correct context is added in @ngtools/webpack, but Ivy doesn't need it at all.
596595
new ContextReplacementPlugin(/\@angular(\\|\/)core(\\|\/)/),
596+
new DedupeModuleResolvePlugin({ verbose: buildOptions.verbose }),
597597
...extraPlugins,
598598
],
599599
};

packages/angular_devkit/build_angular/src/angular-cli-files/plugins/dedupe-module-resolve-plugin.ts

+35-37
Original file line numberDiff line numberDiff line change
@@ -6,89 +6,87 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
interface NormalModuleFactoryRequest {
9+
import { Compiler } from 'webpack';
10+
11+
interface AfterResolveResult {
1012
request: string;
13+
resource: string;
1114
context: {
1215
issuer: string;
1316
};
14-
relativePath: string;
15-
path: string;
16-
descriptionFileData: {
17-
name?: string;
18-
version?: string;
17+
resourceResolveData: {
18+
relativePath: string;
19+
descriptionFileRoot: string;
20+
descriptionFilePath: string;
21+
descriptionFileData: {
22+
name?: string;
23+
version?: string;
24+
};
1925
};
20-
descriptionFileRoot: string;
21-
descriptionFilePath: string;
22-
directory?: boolean;
23-
file?: boolean;
2426
}
2527

2628
export interface DedupeModuleResolvePluginOptions {
2729
verbose?: boolean;
2830
}
2931

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

3944
constructor(private options?: DedupeModuleResolvePluginOptions) { }
4045

4146
// tslint:disable-next-line: no-any
42-
apply(resolver: any) {
43-
resolver
44-
.getHook('before-described-relative')
45-
.tapPromise('DedupeModuleResolvePlugin', async (request: NormalModuleFactoryRequest) => {
46-
if (request.relativePath !== '.') {
47+
apply(compiler: Compiler) {
48+
compiler.hooks.normalModuleFactory.tap('DedupeModuleResolvePlugin', nmf => {
49+
nmf.hooks.afterResolve.tap('DedupeModuleResolvePlugin', (result: AfterResolveResult | undefined) => {
50+
if (!result) {
4751
return;
4852
}
4953

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

56-
// Empty name or versions are no valid primary entrypoints of a library
57-
if (!request.descriptionFileData.name || !request.descriptionFileData.version) {
57+
// Empty name or versions are no valid primary entrypoints of a library
58+
if (!descriptionFileData.name || !descriptionFileData.version) {
5859
return;
5960
}
6061

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

6465
if (!prevResolvedModule) {
6566
// This is the first time we visit this module.
66-
this.modules.set(moduleId, request);
67+
this.modules.set(moduleId, {
68+
resource,
69+
request,
70+
});
6771

6872
return;
6973
}
7074

71-
const {
72-
path,
73-
descriptionFilePath,
74-
descriptionFileRoot,
75-
} = prevResolvedModule;
76-
77-
if (request.path === path) {
75+
const { resource: prevResource, request: prevRequest } = prevResolvedModule;
76+
if (result.resource === prevResource) {
7877
// No deduping needed.
7978
// Current path and previously resolved path are the same.
8079
return;
8180
}
8281

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

8887
// Alter current request with previously resolved module.
89-
request.path = path;
90-
request.descriptionFileRoot = descriptionFileRoot;
91-
request.descriptionFilePath = descriptionFilePath;
88+
result.request = prevRequest;
9289
});
90+
});
9391
}
9492
}

tests/legacy-cli/e2e/tests/misc/dedupe-duplicate-modules.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ export default async function () {
3232
const { stderr } = await ng('build', '--verbose', '--no-vendor-chunk', '--no-progress');
3333
const outFile = 'dist/test-project/main.js';
3434

35-
if (/\[DedupeModuleResolvePlugin\]:.+tslib-1-copy -> .+tslib-1/.test(stderr)) {
35+
if (/\[DedupeModuleResolvePlugin\]:.+tslib-1-copy\/.+ -> .+tslib-1\/.+/.test(stderr)) {
3636
await expectFileToMatch(outFile, './node_modules/tslib-1/tslib.es6.js');
3737
await expectToFail(() => expectFileToMatch(outFile, './node_modules/tslib-1-copy/tslib.es6.js'));
38-
} else if (/\[DedupeModuleResolvePlugin\]:.+tslib-1 -> .+tslib-1-copy/.test(stderr)) {
38+
} else if (/\[DedupeModuleResolvePlugin\]:.+tslib-1\/.+ -> .+tslib-1-copy\/.+/.test(stderr)) {
3939
await expectFileToMatch(outFile, './node_modules/tslib-1-copy/tslib.es6.js');
4040
await expectToFail(() => expectFileToMatch(outFile, './node_modules/tslib-1/tslib.es6.js'));
4141
} else {

0 commit comments

Comments
 (0)