fix(ivy): injectable pipe schematic generating incorrect import statements (#30170)

Currently the injectable pipe schematic generates invalid imports like `import import { Pipe, PipeTransform, Injectable } from '@angular/core'; from '@angular/core';`. The issue wasn't caught by the unit tests, because the invalid import still contains the valid one.

Fixes #30159.

PR Close #30170
This commit is contained in:
crisbeto 2019-04-27 14:00:21 +02:00 committed by Andrew Kushnir
parent 537502d685
commit 6f433887e0
5 changed files with 19 additions and 27 deletions

View File

@ -10,7 +10,7 @@ import {Replacement, RuleFailure, Rules} from 'tslint';
import * as ts from 'typescript'; import * as ts from 'typescript';
import {InjectablePipeVisitor} from '../angular/injectable_pipe_visitor'; import {InjectablePipeVisitor} from '../angular/injectable_pipe_visitor';
import {INJECTABLE_DECORATOR_NAME, addNamedImport, getNamedImports} from '../util'; import {INJECTABLE_DECORATOR_NAME, addImport, getNamedImports} from '../util';
/** /**
* TSLint rule that flags `@Pipe` classes that haven't been marked as `@Injectable`. * TSLint rule that flags `@Pipe` classes that haven't been marked as `@Injectable`.
@ -37,8 +37,7 @@ export class Rule extends Rules.TypedRule {
fixes.push(new Replacement( fixes.push(new Replacement(
namedImports.getStart(), namedImports.getWidth(), namedImports.getStart(), namedImports.getWidth(),
printer.printNode( printer.printNode(
ts.EmitHint.Unspecified, ts.EmitHint.Unspecified, addImport(namedImports, INJECTABLE_DECORATOR_NAME),
addNamedImport(importDeclarationMissingImport, INJECTABLE_DECORATOR_NAME),
sourceFile))); sourceFile)));
} }
} }

View File

@ -14,7 +14,7 @@ import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig'; import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig';
import {InjectablePipeVisitor} from './angular/injectable_pipe_visitor'; import {InjectablePipeVisitor} from './angular/injectable_pipe_visitor';
import {INJECTABLE_DECORATOR_NAME, addNamedImport, getNamedImports} from './util'; import {INJECTABLE_DECORATOR_NAME, addImport, getNamedImports} from './util';
/** /**
* Runs a migration over a TypeScript project that adds an `@Injectable` * Runs a migration over a TypeScript project that adds an `@Injectable`
@ -78,8 +78,7 @@ function runInjectablePipeMigration(tree: Tree, tsconfigPath: string, basePath:
update.insertRight( update.insertRight(
namedImports.getStart(), namedImports.getStart(),
printer.printNode( printer.printNode(
ts.EmitHint.Unspecified, ts.EmitHint.Unspecified, addImport(namedImports, INJECTABLE_DECORATOR_NAME),
addNamedImport(importDeclarationMissingImport, INJECTABLE_DECORATOR_NAME),
sourceFile)); sourceFile));
} }
} }

View File

@ -12,28 +12,17 @@ import * as ts from 'typescript';
export const INJECTABLE_DECORATOR_NAME = 'Injectable'; export const INJECTABLE_DECORATOR_NAME = 'Injectable';
/** /**
* Adds a named import to an import declaration node. * Adds an import to a named import node, if the import does not exist already.
* @param node Node to which to add the import. * @param node Node to which to add the import.
* @param importName Name of the import that should be added. * @param importName Name of the import that should be added.
*/ */
export function addNamedImport(node: ts.ImportDeclaration, importName: string) { export function addImport(node: ts.NamedImports, importName: string) {
const namedImports = getNamedImports(node); const elements = node.elements;
const isAlreadyImported = elements.some(element => element.name.text === importName);
if (namedImports && ts.isNamedImports(namedImports)) { if (!isAlreadyImported) {
const elements = namedImports.elements; return ts.updateNamedImports(
const isAlreadyImported = elements.some(element => element.name.text === importName); node, [...elements, ts.createImportSpecifier(undefined, ts.createIdentifier(importName))]);
if (!isAlreadyImported) {
// If there are named imports, there will be an import clause as well.
const importClause = node.importClause !;
const newImportClause = ts.createNamedImports(
[...elements, ts.createImportSpecifier(undefined, ts.createIdentifier(importName))]);
return ts.updateImportDeclaration(
node, node.decorators, node.modifiers,
ts.updateImportClause(importClause, importClause.name, newImportClause),
node.moduleSpecifier);
}
} }
return node; return node;

View File

@ -85,7 +85,10 @@ describe('Google3 injectable pipe TSLint rule', () => {
`); `);
runTSLint(); runTSLint();
expect(getFile('/index.ts')).toContain('import { Pipe, Injectable } from \'@angular/core\'');
const content = getFile('/index.ts');
expect(content).toContain('import { Pipe, Injectable } from \'@angular/core\'');
expect((content.match(/import/g) || []).length).toBe(1, 'Expected only one import statement');
}); });
}); });

View File

@ -70,8 +70,10 @@ describe('injectable pipe migration', () => {
`); `);
runMigration(); runMigration();
expect(tree.readContent('/index.ts'))
.toContain('import { Pipe, Injectable } from \'@angular/core\''); const content = tree.readContent('/index.ts');
expect(content).toContain('import { Pipe, Injectable } from \'@angular/core\'');
expect((content.match(/import/g) || []).length).toBe(1, 'Expected only one import statement');
}); });
it('should not add an import for Injectable if it is imported already', () => { it('should not add an import for Injectable if it is imported already', () => {