Skip to content

Commit

Permalink
fix(@schematics/angular): correctly handle adding multi-line strings …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
gkalpak authored and filipesilva committed Mar 30, 2021
1 parent 515f042 commit fb14945
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 23 deletions.
4 changes: 2 additions & 2 deletions packages/schematics/angular/component/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,15 +145,15 @@ 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 () => {
const options = { ...defaultOptions, entryComponent: true };

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 () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/schematics/angular/directive/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/schematics/angular/library/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`, () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/schematics/angular/pipe/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
21 changes: 12 additions & 9 deletions packages/schematics/angular/utility/ast-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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}]`;
}
Expand Down Expand Up @@ -404,8 +406,8 @@ export function addSymbolToNgModuleMetadata(

if (Array.isArray(node)) {
const nodeArray = node as {} as Array<ts.Node>;
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 [];
}

Expand All @@ -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}`;
}
Expand Down
12 changes: 6 additions & 6 deletions packages/schematics/angular/utility/ast-utils_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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)', () => {
Expand All @@ -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)', () => {
Expand All @@ -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)', () => {
Expand All @@ -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 '@'`, () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down

0 comments on commit fb14945

Please sign in to comment.