From fb14945c02a3f150d6965e77324416b1ec7cc575 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Fri, 26 Mar 2021 17:55:34 +0200 Subject: [PATCH] fix(@schematics/angular): correctly handle adding multi-line strings to `@NgModule` metadata Previously, `addSymbolToNgModuleMetadata()` assumed that the added symbol would not span multiple lines. In most cases, the added symbol is a single word, so this assumption was correct. In some cases, however, we might want to add a mutli-line string, such as a static method of an `@NgModule`: ```ts imports: [ SomeModule.staticMethod({ prop1: 'val1', prop2: 'val2' }) ] ``` This commit allows `addSymbolToNgModuleMetadata()` to correctly handle multi-line strings by ensuring that added metadata symbols are always put on a new line (even if the array is empty) and each line in the string is indented as necessary. --- .../angular/component/index_spec.ts | 4 ++-- .../angular/directive/index_spec.ts | 2 +- .../schematics/angular/library/index_spec.ts | 2 +- .../schematics/angular/pipe/index_spec.ts | 2 +- .../schematics/angular/utility/ast-utils.ts | 21 +++++++++++-------- .../angular/utility/ast-utils_spec.ts | 12 +++++------ .../component/component-module-export.ts | 2 +- .../directive/directive-module-export.ts | 2 +- .../tests/generate/pipe/pipe-module-export.ts | 2 +- 9 files changed, 26 insertions(+), 23 deletions(-) diff --git a/packages/schematics/angular/component/index_spec.ts b/packages/schematics/angular/component/index_spec.ts index 06f8aaaf203a..3cfd659fd329 100644 --- a/packages/schematics/angular/component/index_spec.ts +++ b/packages/schematics/angular/component/index_spec.ts @@ -145,7 +145,7 @@ describe('Component Schematic', () => { const tree = await schematicRunner.runSchematicAsync('component', options, appTree).toPromise(); const appModuleContent = tree.readContent('/projects/bar/src/app/app.module.ts'); - expect(appModuleContent).toMatch(/exports: \[FooComponent\]/); + expect(appModuleContent).toMatch(/exports: \[\n(\s*) FooComponent\n\1\]/); }); it('should set the entry component', async () => { @@ -153,7 +153,7 @@ describe('Component Schematic', () => { const tree = await schematicRunner.runSchematicAsync('component', options, appTree).toPromise(); const appModuleContent = tree.readContent('/projects/bar/src/app/app.module.ts'); - expect(appModuleContent).toMatch(/entryComponents: \[FooComponent\]/); + expect(appModuleContent).toMatch(/entryComponents: \[\n(\s*) FooComponent\n\1\]/); }); it('should import into a specified module', async () => { diff --git a/packages/schematics/angular/directive/index_spec.ts b/packages/schematics/angular/directive/index_spec.ts index 167afe6bbf4f..b77e976340a1 100644 --- a/packages/schematics/angular/directive/index_spec.ts +++ b/packages/schematics/angular/directive/index_spec.ts @@ -93,7 +93,7 @@ describe('Directive Schematic', () => { const tree = await schematicRunner.runSchematicAsync('directive', options, appTree) .toPromise(); const appModuleContent = tree.readContent('/projects/bar/src/app/app.module.ts'); - expect(appModuleContent).toMatch(/exports: \[FooDirective\]/); + expect(appModuleContent).toMatch(/exports: \[\n(\s*) FooDirective\n\1\]/); }); it('should import into a specified module', async () => { diff --git a/packages/schematics/angular/library/index_spec.ts b/packages/schematics/angular/library/index_spec.ts index cec4bfc3cff5..a6fe99b0def4 100644 --- a/packages/schematics/angular/library/index_spec.ts +++ b/packages/schematics/angular/library/index_spec.ts @@ -139,7 +139,7 @@ describe('Library Schematic', () => { it('should export the component in the NgModule', async () => { const tree = await schematicRunner.runSchematicAsync('library', defaultOptions, workspaceTree).toPromise(); const fileContent = getFileContent(tree, '/projects/foo/src/lib/foo.module.ts'); - expect(fileContent).toContain('exports: [FooComponent]'); + expect(fileContent).toMatch(/exports: \[\n(\s*) FooComponent\n\1\]/); }); describe(`update package.json`, () => { diff --git a/packages/schematics/angular/pipe/index_spec.ts b/packages/schematics/angular/pipe/index_spec.ts index a6178e87885d..263236bd72b4 100644 --- a/packages/schematics/angular/pipe/index_spec.ts +++ b/packages/schematics/angular/pipe/index_spec.ts @@ -99,7 +99,7 @@ describe('Pipe Schematic', () => { const tree = await schematicRunner.runSchematicAsync('pipe', options, appTree).toPromise(); const appModuleContent = getFileContent(tree, '/projects/bar/src/app/app.module.ts'); - expect(appModuleContent).toMatch(/exports: \[FooPipe\]/); + expect(appModuleContent).toMatch(/exports: \[\n(\s*) FooPipe\n\1\]/); }); it('should respect the flat flag', async () => { diff --git a/packages/schematics/angular/utility/ast-utils.ts b/packages/schematics/angular/utility/ast-utils.ts index ac06796fb1f2..63fe5d43603f 100644 --- a/packages/schematics/angular/utility/ast-utils.ts +++ b/packages/schematics/angular/utility/ast-utils.ts @@ -5,6 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import { tags } from '@angular-devkit/core'; import * as ts from '../third_party/github.com/Microsoft/TypeScript/lib/typescript'; import { Change, InsertChange, NoopChange } from './change'; @@ -365,15 +366,16 @@ export function addSymbolToNgModuleMetadata( let toInsert: string; if (expr.properties.length == 0) { position = expr.getEnd() - 1; - toInsert = ` ${metadataField}: [${symbolName}]\n`; + toInsert = `\n ${metadataField}: [\n${tags.indentBy(4)`${symbolName}`}\n ]\n`; } else { node = expr.properties[expr.properties.length - 1]; position = node.getEnd(); // Get the indentation of the last element, if any. const text = node.getFullText(source); - const matches = text.match(/^\r?\n\s*/); - if (matches && matches.length > 0) { - toInsert = `,${matches[0]}${metadataField}: [${symbolName}]`; + const matches = text.match(/^(\r?\n)(\s*)/); + if (matches) { + toInsert = `,${matches[0]}${metadataField}: [${matches[1]}` + + `${tags.indentBy(matches[2].length + 2)`${symbolName}`}${matches[0]}]`; } else { toInsert = `, ${metadataField}: [${symbolName}]`; } @@ -404,8 +406,8 @@ export function addSymbolToNgModuleMetadata( if (Array.isArray(node)) { const nodeArray = node as {} as Array; - const symbolsArray = nodeArray.map(node => node.getText()); - if (symbolsArray.includes(symbolName)) { + const symbolsArray = nodeArray.map(node => tags.oneLine`${node.getText()}`); + if (symbolsArray.includes(tags.oneLine`${symbolName}`)) { return []; } @@ -417,12 +419,13 @@ export function addSymbolToNgModuleMetadata( if (node.kind == ts.SyntaxKind.ArrayLiteralExpression) { // We found the field but it's empty. Insert it just before the `]`. position--; - toInsert = `${symbolName}`; + toInsert = `\n${tags.indentBy(4)`${symbolName}`}\n `; } else { // Get the indentation of the last element, if any. const text = node.getFullText(source); - if (text.match(/^\r?\n/)) { - toInsert = `,${text.match(/^\r?\n(\r?)\s*/)[0]}${symbolName}`; + const matches = text.match(/^(\r?\n)(\s*)/); + if (matches) { + toInsert = `,${matches[1]}${tags.indentBy(matches[2].length)`${symbolName}`}`; } else { toInsert = `, ${symbolName}`; } diff --git a/packages/schematics/angular/utility/ast-utils_spec.ts b/packages/schematics/angular/utility/ast-utils_spec.ts index 7f58932ac970..286a757df47e 100644 --- a/packages/schematics/angular/utility/ast-utils_spec.ts +++ b/packages/schematics/angular/utility/ast-utils_spec.ts @@ -70,7 +70,7 @@ describe('ast utils', () => { const changes = addExportToModule(source, modulePath, 'FooComponent', './foo.component'); const output = applyChanges(modulePath, moduleContent, changes); expect(output).toMatch(/import { FooComponent } from '.\/foo.component';/); - expect(output).toMatch(/exports: \[FooComponent\]/); + expect(output).toMatch(/exports: \[\n(\s*) FooComponent\n\1\]/); }); it('should add export to module if not indented', () => { @@ -79,7 +79,7 @@ describe('ast utils', () => { const changes = addExportToModule(source, modulePath, 'FooComponent', './foo.component'); const output = applyChanges(modulePath, moduleContent, changes); expect(output).toMatch(/import { FooComponent } from '.\/foo.component';/); - expect(output).toMatch(/exports: \[FooComponent\]/); + expect(output).toMatch(/exports: \[\n(\s*) FooComponent\n\1\]/); }); it('should add declarations to module if not indented', () => { @@ -120,7 +120,7 @@ describe('ast utils', () => { expect(changes).not.toBeNull(); const output = applyChanges(modulePath, moduleContent, changes || []); - expect(output).toMatch(/imports: [\s\S]+,\n\s+HelloWorld\n\s+\]/m); + expect(output).toMatch(/imports: \[[^\]]+,\n(\s*) HelloWorld\n\1\]/); }); it('should add metadata (comma)', () => { @@ -145,7 +145,7 @@ describe('ast utils', () => { expect(changes).not.toBeNull(); const output = applyChanges(modulePath, moduleContent, changes || []); - expect(output).toMatch(/imports: [\s\S]+,\n\s+HelloWorld,\n\s+\]/m); + expect(output).toMatch(/imports: \[[^\]]+,\n(\s*) HelloWorld,\n\1\]/); }); it('should add metadata (missing)', () => { @@ -167,7 +167,7 @@ describe('ast utils', () => { expect(changes).not.toBeNull(); const output = applyChanges(modulePath, moduleContent, changes || []); - expect(output).toMatch(/imports: \[HelloWorld]\r?\n/m); + expect(output).toMatch(/imports: \[\n(\s*) HelloWorld\n\1\]/); }); it('should add metadata (empty)', () => { @@ -190,7 +190,7 @@ describe('ast utils', () => { expect(changes).not.toBeNull(); const output = applyChanges(modulePath, moduleContent, changes || []); - expect(output).toMatch(/imports: \[HelloWorld],\r?\n/m); + expect(output).toMatch(/imports: \[\n(\s*) HelloWorld\n\1\],\n/); }); it(`should handle NgModule with newline after '@'`, () => { diff --git a/tests/legacy-cli/e2e/tests/generate/component/component-module-export.ts b/tests/legacy-cli/e2e/tests/generate/component/component-module-export.ts index c17e7f051468..dbe45583f172 100644 --- a/tests/legacy-cli/e2e/tests/generate/component/component-module-export.ts +++ b/tests/legacy-cli/e2e/tests/generate/component/component-module-export.ts @@ -7,7 +7,7 @@ export default function() { const modulePath = join('src', 'app', 'app.module.ts'); return ng('generate', 'component', 'test-component', '--export') - .then(() => expectFileToMatch(modulePath, 'exports: [TestComponentComponent]')) + .then(() => expectFileToMatch(modulePath, /exports: \[\n(\s*) TestComponentComponent\n\1\]/)) // Try to run the unit tests. .then(() => ng('test', '--watch=false')); diff --git a/tests/legacy-cli/e2e/tests/generate/directive/directive-module-export.ts b/tests/legacy-cli/e2e/tests/generate/directive/directive-module-export.ts index 2e1499f2d0e2..ecc34dfaecc4 100644 --- a/tests/legacy-cli/e2e/tests/generate/directive/directive-module-export.ts +++ b/tests/legacy-cli/e2e/tests/generate/directive/directive-module-export.ts @@ -7,7 +7,7 @@ export default function() { const modulePath = join('src', 'app', 'app.module.ts'); return ng('generate', 'directive', 'test-directive', '--export') - .then(() => expectFileToMatch(modulePath, 'exports: [TestDirectiveDirective]')) + .then(() => expectFileToMatch(modulePath, /exports: \[\n(\s*) TestDirectiveDirective\n\1\]/)) // Try to run the unit tests. .then(() => ng('test', '--watch=false')); diff --git a/tests/legacy-cli/e2e/tests/generate/pipe/pipe-module-export.ts b/tests/legacy-cli/e2e/tests/generate/pipe/pipe-module-export.ts index 570f7b339bcb..fc82781d97fb 100644 --- a/tests/legacy-cli/e2e/tests/generate/pipe/pipe-module-export.ts +++ b/tests/legacy-cli/e2e/tests/generate/pipe/pipe-module-export.ts @@ -7,7 +7,7 @@ export default function() { const modulePath = join('src', 'app', 'app.module.ts'); return ng('generate', 'pipe', 'test-pipe', '--export') - .then(() => expectFileToMatch(modulePath, 'exports: [TestPipePipe]')) + .then(() => expectFileToMatch(modulePath, /exports: \[\n(\s*) TestPipePipe\n\1\]/)) // Try to run the unit tests. .then(() => ng('test', '--watch=false'));