Skip to content

Commit 82ddc5c

Browse files
committed
fix(@ngtools/webpack): only remove moduleId in decorators
Prior to this we were removing all mentions of moduleId, which is invalid if the users want to use it themselves. Now we only removes it if its in a decorator. There might be a slight regression with people using static const objects instead of object literals in their decorators but this shuold not happen often and even less often with moduleId. Fixes #5509.
1 parent 7087195 commit 82ddc5c

File tree

2 files changed

+23
-7
lines changed

2 files changed

+23
-7
lines changed

packages/@ngtools/webpack/src/loader.spec.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,28 @@ describe('@ngtools/webpack', () => {
1111
host.writeFile('/file.ts', `
1212
export const obj = { moduleId: 123 };
1313
export const obj2 = { moduleId: 123, otherValue: 1 };
14-
export const obj2 = { otherValue: 1, moduleId: 123 };
14+
export const obj3 = { otherValue: 1, moduleId: 123 };
15+
`, false);
16+
host.writeFile('/file2.ts', `
17+
@SomeDecorator({ moduleId: 123 }) class CLS {}
18+
@SomeDecorator({ moduleId: 123, otherValue1: 1 }) class CLS2 {}
19+
@SomeDecorator({ otherValue2: 2, moduleId: 123, otherValue3: 3 }) class CLS3 {}
20+
@SomeDecorator({ otherValue4: 4, moduleId: 123 }) class CLS4 {}
1521
`, false);
1622

17-
const program = ts.createProgram(['/file.ts'], {}, host);
23+
const program = ts.createProgram(['/file.ts', '/file2.ts'], {}, host);
1824

1925
const refactor = new TypeScriptFileRefactor('/file.ts', host, program);
2026
removeModuleIdOnlyForTesting(refactor);
27+
expect(refactor.sourceText).not.toMatch(/obj = \{\s+};/);
28+
expect(refactor.sourceText).not.toMatch(/\{\s*otherValue: 1\s*};/);
2129

22-
expect(refactor.sourceText).toMatch(/obj = \{\s+};/);
23-
expect(refactor.sourceText).toMatch(/obj2 = \{\s*otherValue: 1\s*};/);
30+
const refactor2 = new TypeScriptFileRefactor('/file2.ts', host, program);
31+
removeModuleIdOnlyForTesting(refactor2);
32+
expect(refactor2.sourceText).toMatch(/\(\{\s+}\)/);
33+
expect(refactor2.sourceText).toMatch(/\(\{\s*otherValue1: 1\s*}\)/);
34+
expect(refactor2.sourceText).toMatch(/\(\{\s*otherValue2: 2\s*,\s*otherValue3: 3\s*}\)/);
35+
expect(refactor2.sourceText).toMatch(/\(\{\s*otherValue4: 4\s*}\)/);
2436
});
2537
});
2638
});

packages/@ngtools/webpack/src/loader.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,10 @@ export function removeModuleIdOnlyForTesting(refactor: TypeScriptFileRefactor) {
241241
function _removeModuleId(refactor: TypeScriptFileRefactor) {
242242
const sourceFile = refactor.sourceFile;
243243

244-
refactor.findAstNodes(sourceFile, ts.SyntaxKind.ObjectLiteralExpression, true)
244+
refactor.findAstNodes(sourceFile, ts.SyntaxKind.Decorator, true)
245+
.reduce((acc, node) => {
246+
return acc.concat(refactor.findAstNodes(node, ts.SyntaxKind.ObjectLiteralExpression, true));
247+
}, [])
245248
// Get all their property assignments.
246249
.filter((node: ts.ObjectLiteralExpression) => {
247250
return node.properties.some(prop => {
@@ -254,8 +257,9 @@ function _removeModuleId(refactor: TypeScriptFileRefactor) {
254257
return prop.kind == ts.SyntaxKind.PropertyAssignment
255258
&& _getContentOfKeyLiteral(sourceFile, prop.name) == 'moduleId';
256259
})[0];
257-
// get the trailing comma
258-
const moduleIdCommaProp = moduleIdProp.parent.getChildAt(1).getChildren()[1];
260+
// Get the trailing comma.
261+
const moduleIdCommaProp = moduleIdProp.parent
262+
? moduleIdProp.parent.getChildAt(1).getChildren()[1] : null;
259263
refactor.removeNodes(moduleIdProp, moduleIdCommaProp);
260264
});
261265
}

0 commit comments

Comments
 (0)