From 1216c0dbaed51d219d836a3588305e4fa4c6d0eb Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sun, 19 Mar 2017 11:34:44 -0700 Subject: [PATCH 1/2] 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. --- packages/@ngtools/webpack/src/loader.spec.ts | 20 ++++++++++++++++---- packages/@ngtools/webpack/src/loader.ts | 10 +++++++--- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/@ngtools/webpack/src/loader.spec.ts b/packages/@ngtools/webpack/src/loader.spec.ts index 7e39c2c8224a..ea356f5ae755 100644 --- a/packages/@ngtools/webpack/src/loader.spec.ts +++ b/packages/@ngtools/webpack/src/loader.spec.ts @@ -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*}\)/); }); }); }); diff --git a/packages/@ngtools/webpack/src/loader.ts b/packages/@ngtools/webpack/src/loader.ts index 57197dd905be..b297c8b18f38 100644 --- a/packages/@ngtools/webpack/src/loader.ts +++ b/packages/@ngtools/webpack/src/loader.ts @@ -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 => { @@ -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); }); } From 3d322cf68a7c3fc69818bc39341d9f3eba12e8cb Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sun, 19 Mar 2017 12:19:12 -0700 Subject: [PATCH 2/2] fix(@ngtools/webpack): add parent nodes and keep program Technically that program should always be the valid one, and is needed in some cases (e.g. diagnostics). Adding parent nodes get rid of the getChildAt error that happens, because the prop.parent is not set. The issue happened because people are using "files": ["main.ts"] or something similar, and when we load another file than main we dont set the parent nodes. Fixes #5143 Fixes #4817 --- packages/@ngtools/webpack/src/loader.spec.ts | 21 ++++++++++++++++++++ packages/@ngtools/webpack/src/refactor.ts | 3 +-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/@ngtools/webpack/src/loader.spec.ts b/packages/@ngtools/webpack/src/loader.spec.ts index ea356f5ae755..5514dd342f7f 100644 --- a/packages/@ngtools/webpack/src/loader.spec.ts +++ b/packages/@ngtools/webpack/src/loader.spec.ts @@ -34,6 +34,27 @@ describe('@ngtools/webpack', () => { expect(refactor2.sourceText).toMatch(/\(\{\s*otherValue2: 2\s*,\s*otherValue3: 3\s*}\)/); expect(refactor2.sourceText).toMatch(/\(\{\s*otherValue4: 4\s*}\)/); }); + + it('should work without a root name', () => { + const host = new WebpackCompilerHost({}, ''); + host.writeFile('/file.ts', ` + import './file2.ts'; + `, 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 refactor = new TypeScriptFileRefactor('/file2.ts', host, program); + removeModuleIdOnlyForTesting(refactor); + expect(refactor.sourceText).toMatch(/\(\{\s+}\)/); + expect(refactor.sourceText).toMatch(/\(\{\s*otherValue1: 1\s*}\)/); + expect(refactor.sourceText).toMatch(/\(\{\s*otherValue2: 2\s*,\s*otherValue3: 3\s*}\)/); + expect(refactor.sourceText).toMatch(/\(\{\s*otherValue4: 4\s*}\)/); + }); }); }); }); diff --git a/packages/@ngtools/webpack/src/refactor.ts b/packages/@ngtools/webpack/src/refactor.ts index 7b11da71d3a1..25c85f946415 100644 --- a/packages/@ngtools/webpack/src/refactor.ts +++ b/packages/@ngtools/webpack/src/refactor.ts @@ -45,9 +45,8 @@ export class TypeScriptFileRefactor { this._sourceFile = _program.getSourceFile(fileName); } if (!this._sourceFile) { - this._program = null; this._sourceFile = ts.createSourceFile(fileName, _host.readFile(fileName), - ts.ScriptTarget.Latest); + ts.ScriptTarget.Latest, true); } this._sourceText = this._sourceFile.getFullText(this._sourceFile); this._sourceString = new MagicString(this._sourceText);