refactor(core): remove disabled injectable-pipe migration (#32184)

Initially the plan was to have a migration that adds `@Injectable()` to
all pipes in a CLI project so that the pipes can be injected in Ivy
similarly to how it worked in view engine.

Due to the planned refactorings which ensure that `@Directive`, `@Component`
and `@Pipe` also have a factory definition, this migration is no longer
needed for Ivy. Additionally since it is already disabled (due to
572b54967c) and we have a more generic
migration (known as `missing-injectable)` that could do the same as
`injectable-pipe`, we remove the migration from the code-base.

PR Close #32184
This commit is contained in:
Paul Gschwendtner 2019-08-19 10:55:38 +02:00 committed by Andrew Kushnir
parent 5da5ca5c23
commit 639b732024
12 changed files with 0 additions and 533 deletions

View File

@ -10,7 +10,6 @@ npm_package(
srcs = ["migrations.json"],
visibility = ["//packages/core:__pkg__"],
deps = [
"//packages/core/schematics/migrations/injectable-pipe",
"//packages/core/schematics/migrations/move-document",
"//packages/core/schematics/migrations/renderer-to-renderer2",
"//packages/core/schematics/migrations/static-queries",

View File

@ -6,7 +6,6 @@ ts_library(
tsconfig = "//packages/core/schematics:tsconfig.json",
visibility = ["//packages/core/schematics/test/google3:__pkg__"],
deps = [
"//packages/core/schematics/migrations/injectable-pipe",
"//packages/core/schematics/migrations/missing-injectable",
"//packages/core/schematics/migrations/missing-injectable/google3",
"//packages/core/schematics/migrations/renderer-to-renderer2",

View File

@ -1,55 +0,0 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* 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 {Replacement, RuleFailure, Rules} from 'tslint';
import * as ts from 'typescript';
import {InjectablePipeVisitor} from '../injectable-pipe/angular/injectable_pipe_visitor';
import {INJECTABLE_DECORATOR_NAME, addImport, getNamedImports} from '../injectable-pipe/util';
/**
* TSLint rule that flags `@Pipe` classes that haven't been marked as `@Injectable`.
*/
export class Rule extends Rules.TypedRule {
applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] {
const visitor = new InjectablePipeVisitor(program.getTypeChecker());
const printer = ts.createPrinter();
const failures: RuleFailure[] = [];
visitor.visitNode(sourceFile);
visitor.missingInjectablePipes.forEach(data => {
const {pipeDecorator, importDeclarationMissingImport} = data;
const fixes = [new Replacement(
pipeDecorator.getStart(), pipeDecorator.getWidth(),
`@${INJECTABLE_DECORATOR_NAME}()\n${pipeDecorator.getText()}`)];
if (importDeclarationMissingImport) {
const namedImports = getNamedImports(importDeclarationMissingImport);
// Add another fix that'll add the missing import.
if (namedImports) {
fixes.push(new Replacement(
namedImports.getStart(), namedImports.getWidth(),
printer.printNode(
ts.EmitHint.Unspecified, addImport(namedImports, INJECTABLE_DECORATOR_NAME),
sourceFile)));
}
}
// Add a failure on Pipe decorators that are missing the Injectable decorator.
failures.push(new RuleFailure(
sourceFile, pipeDecorator.getStart(), pipeDecorator.getWidth(),
'Classes with @Pipe should be decorated with @Injectable so that they can be injected.',
this.ruleName, fixes));
});
return failures;
}
}

View File

@ -1,18 +0,0 @@
load("//tools:defaults.bzl", "ts_library")
ts_library(
name = "injectable-pipe",
srcs = glob(["**/*.ts"]),
tsconfig = "//packages/core/schematics:tsconfig.json",
visibility = [
"//packages/core/schematics:__pkg__",
"//packages/core/schematics/migrations/google3:__pkg__",
"//packages/core/schematics/test:__pkg__",
],
deps = [
"//packages/core/schematics/utils",
"@npm//@angular-devkit/schematics",
"@npm//@types/node",
"@npm//typescript",
],
)

View File

@ -1,22 +0,0 @@
## Injectable annotation on pipes
In ViewEngine it was possible to inject a class that was annotated as a `Pipe`, however this no
longer works in Ivy if the class also doesn't have the `Injectable` decorator. This migration
adds `Injectable` automatically to all `Pipe` classes.
### Before
```ts
import { Pipe } from '@angular/core';
@Pipe({ name: 'myPipe' })
class MyPipe {}
```
### After
```ts
import { Pipe, Injectable } from '@angular/core';
@Injectable()
@Pipe({ name: 'myPipe' })
class MyPipe {}
```

View File

@ -1,67 +0,0 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* 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 * as ts from 'typescript';
import {getAngularDecorators} from '../../../utils/ng_decorators';
import {INJECTABLE_DECORATOR_NAME} from '../util';
/**
* Goes through all of the descendant nodes of a given node and lists out all of the pipes
* that don't have `@Injectable`, as well as their `@Pipe` decorator and the import declaration
* from which we'd need to import the `Injectable` decorator.
*/
export class InjectablePipeVisitor {
/**
* Keeps track of all the classes that have a `Pipe` decorator, but not `Injectable`, as well
* as a reference to the `Pipe` decorator itself and import declarations from which we'll have
* to import the `Injectable` decorator.
*/
missingInjectablePipes: {
classDeclaration: ts.ClassDeclaration,
importDeclarationMissingImport: ts.ImportDeclaration|null,
pipeDecorator: ts.Decorator
}[] = [];
constructor(private _typeChecker: ts.TypeChecker) {}
visitNode(node: ts.Node) {
switch (node.kind) {
case ts.SyntaxKind.ClassDeclaration:
this._visitClassDeclaration(node as ts.ClassDeclaration);
break;
}
ts.forEachChild(node, node => this.visitNode(node));
}
private _visitClassDeclaration(node: ts.ClassDeclaration) {
if (!node.decorators || !node.decorators.length) {
return;
}
const ngDecorators = getAngularDecorators(this._typeChecker, node.decorators);
const pipeDecorator = ngDecorators.find(decorator => decorator.name === 'Pipe');
const hasInjectableDecorator =
!ngDecorators.some(decorator => decorator.name === INJECTABLE_DECORATOR_NAME);
// Skip non-pipe classes and pipes that are already marked as injectable.
if (pipeDecorator && hasInjectableDecorator) {
const importNode = pipeDecorator.importNode;
const namedImports = importNode.importClause && importNode.importClause.namedBindings;
const needsImport = namedImports && ts.isNamedImports(namedImports) &&
!namedImports.elements.some(element => element.name.text === INJECTABLE_DECORATOR_NAME);
this.missingInjectablePipes.push({
classDeclaration: node,
importDeclarationMissingImport: needsImport ? importNode : null,
pipeDecorator: pipeDecorator.node
});
}
}
}

View File

@ -1,92 +0,0 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* 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 {Rule, SchematicsException, Tree} from '@angular-devkit/schematics';
import {dirname, relative} from 'path';
import * as ts from 'typescript';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig';
import {InjectablePipeVisitor} from './angular/injectable_pipe_visitor';
import {INJECTABLE_DECORATOR_NAME, addImport, getNamedImports} from './util';
/**
* Runs a migration over a TypeScript project that adds an `@Injectable`
* annotation to all classes that have `@Pipe`.
*/
export default function(): Rule {
return (tree: Tree) => {
const {buildPaths, testPaths} = getProjectTsConfigPaths(tree);
const basePath = process.cwd();
const allPaths = [...buildPaths, ...testPaths];
if (!allPaths.length) {
throw new SchematicsException(
'Could not find any tsconfig file. Cannot add Injectable annotation to pipes.');
}
for (const tsconfigPath of allPaths) {
runInjectablePipeMigration(tree, tsconfigPath, basePath);
}
};
}
function runInjectablePipeMigration(tree: Tree, tsconfigPath: string, basePath: string) {
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
const host = ts.createCompilerHost(parsed.options, true);
// We need to overwrite the host "readFile" method, as we want the TypeScript
// program to be based on the file contents in the virtual file tree. Otherwise
// if we run the migration for multiple tsconfig files which have intersecting
// source files, it can end up updating query definitions multiple times.
host.readFile = fileName => {
const buffer = tree.read(relative(basePath, fileName));
// Strip BOM as otherwise TSC methods (Ex: getWidth) will return an offset which
// which breaks the CLI UpdateRecorder.
// See: https://github.com/angular/angular/pull/30719
return buffer ? buffer.toString().replace(/^\uFEFF/, '') : undefined;
};
const program = ts.createProgram(parsed.fileNames, parsed.options, host);
const typeChecker = program.getTypeChecker();
const visitor = new InjectablePipeVisitor(typeChecker);
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const printer = ts.createPrinter();
sourceFiles.forEach(sourceFile => visitor.visitNode(sourceFile));
visitor.missingInjectablePipes.forEach(data => {
const {classDeclaration, importDeclarationMissingImport} = data;
const sourceFile = classDeclaration.getSourceFile();
const update = tree.beginUpdate(relative(basePath, sourceFile.fileName));
// Note that we don't need to go through the AST to insert the decorator, because the change
// is pretty basic. Also this has a better chance of preserving the user's formatting.
update.insertLeft(classDeclaration.getStart(), `@${INJECTABLE_DECORATOR_NAME}()\n`);
// Add @Injectable to the imports if it isn't imported already. Note that this doesn't deal with
// the case where there aren't any imports for `@angular/core` at all. We don't need to handle
// it because the Pipe decorator won't be recognized if it hasn't been imported from Angular.
if (importDeclarationMissingImport) {
const namedImports = getNamedImports(importDeclarationMissingImport);
if (namedImports) {
update.remove(namedImports.getStart(), namedImports.getWidth());
update.insertRight(
namedImports.getStart(),
printer.printNode(
ts.EmitHint.Unspecified, addImport(namedImports, INJECTABLE_DECORATOR_NAME),
sourceFile));
}
}
tree.commitUpdate(update);
});
}

View File

@ -1,36 +0,0 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* 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 * as ts from 'typescript';
/** Name of the Injectable decorator. */
export const INJECTABLE_DECORATOR_NAME = 'Injectable';
/**
* 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 importName Name of the import that should be added.
*/
export function addImport(node: ts.NamedImports, importName: string) {
const elements = node.elements;
const isAlreadyImported = elements.some(element => element.name.text === importName);
if (!isAlreadyImported) {
return ts.updateNamedImports(
node, [...elements, ts.createImportSpecifier(undefined, ts.createIdentifier(importName))]);
}
return node;
}
/** Gets the named imports node from an import declaration. */
export function getNamedImports(node: ts.ImportDeclaration): ts.NamedImports|null {
const importClause = node.importClause;
const namedImports = importClause && importClause.namedBindings;
return (namedImports && ts.isNamedImports(namedImports)) ? namedImports : null;
}

View File

@ -9,7 +9,6 @@ ts_library(
"//packages/core/schematics:migrations.json",
],
deps = [
"//packages/core/schematics/migrations/injectable-pipe",
"//packages/core/schematics/migrations/missing-injectable",
"//packages/core/schematics/migrations/move-document",
"//packages/core/schematics/migrations/renderer-to-renderer2",

View File

@ -1,93 +0,0 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* 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 {readFileSync, writeFileSync} from 'fs';
import {dirname, join} from 'path';
import * as shx from 'shelljs';
import {Configuration, Linter} from 'tslint';
describe('Google3 injectable pipe TSLint rule', () => {
const rulesDirectory = dirname(require.resolve('../../migrations/google3/injectablePipeRule'));
let tmpDir: string;
beforeEach(() => {
tmpDir = join(process.env['TEST_TMPDIR'] !, 'google3-test');
shx.mkdir('-p', tmpDir);
writeFile('tsconfig.json', JSON.stringify({compilerOptions: {module: 'es2015'}}));
});
afterEach(() => shx.rm('-r', tmpDir));
function runTSLint(fix = true) {
const program = Linter.createProgram(join(tmpDir, 'tsconfig.json'));
const linter = new Linter({fix, rulesDirectory: [rulesDirectory]}, program);
const config = Configuration.parseConfigFile(
{rules: {'injectable-pipe': true}, linterOptions: {typeCheck: true}});
program.getRootFileNames().forEach(fileName => {
linter.lint(fileName, program.getSourceFile(fileName) !.getFullText(), config);
});
return linter;
}
function writeFile(fileName: string, content: string) {
writeFileSync(join(tmpDir, fileName), content);
}
function getFile(fileName: string) { return readFileSync(join(tmpDir, fileName), 'utf8'); }
it('should report pipes that are not marked as Injectable', () => {
writeFile('index.ts', `
import { Pipe } from '@angular/core';
@Pipe({ name: 'myPipe' })
export class MyPipe {
}
`);
const linter = runTSLint(false);
const failures = linter.getResult().failures;
expect(failures.length).toBe(1);
expect(failures[0].getFailure()).toMatch(/@Pipe should be decorated with @Injectable/);
});
it('should add @Injectable to pipes that do not have it', () => {
writeFile('/index.ts', `
import { Pipe } from '@angular/core';
@Pipe({ name: 'myPipe' })
export class MyPipe {
}
`);
runTSLint();
expect(getFile('/index.ts'))
.toMatch(/@Injectable\(\)\s+@Pipe\(\{ name: 'myPipe' \}\)\s+export class MyPipe/);
});
it('should add an import for Injectable to the @angular/core import declaration', () => {
writeFile('/index.ts', `
import { Pipe } from '@angular/core';
@Pipe()
export class MyPipe {
}
`);
runTSLint();
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

@ -1,143 +0,0 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* 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 {getSystemPath, normalize, virtualFs} from '@angular-devkit/core';
import {TempScopedNodeJsSyncHost} from '@angular-devkit/core/node/testing';
import {HostTree} from '@angular-devkit/schematics';
import {SchematicTestRunner, UnitTestTree} from '@angular-devkit/schematics/testing';
import * as shx from 'shelljs';
describe('injectable pipe migration', () => {
let runner: SchematicTestRunner;
let host: TempScopedNodeJsSyncHost;
let tree: UnitTestTree;
let tmpDirPath: string;
let previousWorkingDir: string;
beforeEach(() => {
runner = new SchematicTestRunner('test', require.resolve('./test-migrations.json'));
host = new TempScopedNodeJsSyncHost();
tree = new UnitTestTree(new HostTree(host));
writeFile('/tsconfig.json', JSON.stringify({
compilerOptions: {
lib: ['es2015'],
}
}));
writeFile('/angular.json', JSON.stringify({
projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}}
}));
previousWorkingDir = shx.pwd();
tmpDirPath = getSystemPath(host.root);
// Switch into the temporary directory path. This allows us to run
// the schematic against our custom unit test tree.
shx.cd(tmpDirPath);
});
afterEach(() => {
shx.cd(previousWorkingDir);
shx.rm('-r', tmpDirPath);
});
it('should add @Injectable to pipes that do not have it', async() => {
writeFile('/index.ts', `
import { Pipe } from '@angular/core';
@Pipe({ name: 'myPipe' })
export class MyPipe {
}
`);
await runMigration();
expect(tree.readContent('/index.ts'))
.toMatch(/@Injectable\(\)\s+@Pipe\(\{ name: 'myPipe' \}\)\s+export class MyPipe/);
});
it('should add @Injectable to pipes that do not have it (BOM)', () => {
writeFile('/index.ts', `\uFEFF
import { Pipe } from '@angular/core';
@Pipe({ name: 'myPipe' })
export class MyPipe {
}
`);
runMigration();
expect(tree.readContent('/index.ts'))
.toMatch(/@Injectable\(\)\s+@Pipe\(\{ name: 'myPipe' \}\)\s+export class MyPipe/);
});
it('should add an import for Injectable to the @angular/core import declaration', async() => {
writeFile('/index.ts', `
import { Pipe } from '@angular/core';
@Pipe()
export class MyPipe {
}
`);
await runMigration();
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', async() => {
writeFile('/index.ts', `
import { Pipe, Injectable, NgModule } from '@angular/core';
@Pipe()
export class MyPipe {
}
`);
await runMigration();
expect(tree.readContent('/index.ts'))
.toContain('import { Pipe, Injectable, NgModule } from \'@angular/core\'');
});
it('should do nothing if the pipe is marked as injectable already', async() => {
const source = `
import { Injectable, Pipe } from '@angular/core';
@Injectable()
@Pipe()
export class MyPipe {
}
`;
writeFile('/index.ts', source);
await runMigration();
expect(tree.readContent('/index.ts')).toBe(source);
});
it('should not add @Injectable if @Pipe was not imported from @angular/core', async() => {
const source = `
import { Pipe } from '@not-angular/core';
@Pipe()
export class MyPipe {
}
`;
writeFile('/index.ts', source);
await runMigration();
expect(tree.readContent('/index.ts')).toBe(source);
});
function writeFile(filePath: string, contents: string) {
host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents));
}
function runMigration() {
runner.runSchematicAsync('migration-injectable-pipe', {}, tree).toPromise();
}
});

View File

@ -2,10 +2,6 @@
// Migrations which are not publicly enabled but still run as part of tests need to
// be part of a schematic collection in order to be able to run it.
"schematics": {
"migration-injectable-pipe": {
"description": "Migrates all Pipe classes so that they have an Injectable annotation",
"factory": "../migrations/injectable-pipe/index"
},
"migration-missing-injectable": {
"description": "Migrates all declared undecorated providers with the @Injectable decorator",
"factory": "../migrations/missing-injectable/index"