Skip to content

Commit

Permalink
fix(@ngtools/webpack): only remove moduleId in decorators
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hansl committed Mar 20, 2017
1 parent 7087195 commit 82ddc5c
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 7 deletions.
20 changes: 16 additions & 4 deletions packages/@ngtools/webpack/src/loader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,28 @@ describe('@ngtools/webpack', () => {
host.writeFile('/file.ts', `
export const obj = { moduleId: 123 };
export const obj2 = { moduleId: 123, otherValue: 1 };
export const obj2 = { otherValue: 1, moduleId: 123 };
export const obj3 = { otherValue: 1, moduleId: 123 };
`, false);
host.writeFile('/file2.ts', `
@SomeDecorator({ moduleId: 123 }) class CLS {}
@SomeDecorator({ moduleId: 123, otherValue1: 1 }) class CLS2 {}
@SomeDecorator({ otherValue2: 2, moduleId: 123, otherValue3: 3 }) class CLS3 {}
@SomeDecorator({ otherValue4: 4, moduleId: 123 }) class CLS4 {}
`, false);

const program = ts.createProgram(['/file.ts'], {}, host);
const program = ts.createProgram(['/file.ts', '/file2.ts'], {}, host);

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

expect(refactor.sourceText).toMatch(/obj = \{\s+};/);
expect(refactor.sourceText).toMatch(/obj2 = \{\s*otherValue: 1\s*};/);
const refactor2 = new TypeScriptFileRefactor('/file2.ts', host, program);
removeModuleIdOnlyForTesting(refactor2);
expect(refactor2.sourceText).toMatch(/\(\{\s+}\)/);
expect(refactor2.sourceText).toMatch(/\(\{\s*otherValue1: 1\s*}\)/);
expect(refactor2.sourceText).toMatch(/\(\{\s*otherValue2: 2\s*,\s*otherValue3: 3\s*}\)/);
expect(refactor2.sourceText).toMatch(/\(\{\s*otherValue4: 4\s*}\)/);
});
});
});
Expand Down
10 changes: 7 additions & 3 deletions packages/@ngtools/webpack/src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,10 @@ export function removeModuleIdOnlyForTesting(refactor: TypeScriptFileRefactor) {
function _removeModuleId(refactor: TypeScriptFileRefactor) {
const sourceFile = refactor.sourceFile;

refactor.findAstNodes(sourceFile, ts.SyntaxKind.ObjectLiteralExpression, true)
refactor.findAstNodes(sourceFile, ts.SyntaxKind.Decorator, true)
.reduce((acc, node) => {
return acc.concat(refactor.findAstNodes(node, ts.SyntaxKind.ObjectLiteralExpression, true));
}, [])
// Get all their property assignments.
.filter((node: ts.ObjectLiteralExpression) => {
return node.properties.some(prop => {
Expand All @@ -254,8 +257,9 @@ function _removeModuleId(refactor: TypeScriptFileRefactor) {
return prop.kind == ts.SyntaxKind.PropertyAssignment
&& _getContentOfKeyLiteral(sourceFile, prop.name) == 'moduleId';
})[0];
// get the trailing comma
const moduleIdCommaProp = moduleIdProp.parent.getChildAt(1).getChildren()[1];
// Get the trailing comma.
const moduleIdCommaProp = moduleIdProp.parent
? moduleIdProp.parent.getChildAt(1).getChildren()[1] : null;
refactor.removeNodes(moduleIdProp, moduleIdCommaProp);
});
}
Expand Down

0 comments on commit 82ddc5c

Please sign in to comment.