Skip to content

Commit

Permalink
fixup! feat(core): add schematics to move deprecated DOCUMENT import
Browse files Browse the repository at this point in the history
  • Loading branch information
@CaerusKaru
CaerusKaru committed Apr 18, 2019
1 parent 6cff935 commit 7327824
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 41 deletions.
17 changes: 9 additions & 8 deletions packages/core/schematics/migrations/move-document/document_import_visitor.ts
View file
Expand Up @@ -13,24 +13,23 @@ const PLATFORM_BROWSER_IMPORT = '@angular/platform-browser';
export const DOCUMENT_TOKEN_NAME = 'DOCUMENT';

export interface Imports {
platformBrowserImport: ts.NamedImports | null;
commonImport: ts.NamedImports | null;
replaceText: string | null;
documentElement: ts.ImportSpecifier | null;
platformBrowserImport: ts.NamedImports|null;
commonImport: ts.NamedImports|null;
replaceText: string|null;
documentElement: ts.ImportSpecifier|null;
}

/**
* Visitor that can be used to find a set of imports in a TypeScript file.
*/
export class DocumentImportVisitor {

importsMap: Map<ts.SourceFile, Imports> = new Map();

constructor(public typeChecker: ts.TypeChecker) {}

visitNode(node: ts.Node) {
if (ts.isNamedImports(node)) {
this.visitNamedImport(node);
this.visitNamedImport(node);
}

ts.forEachChild(node, node => this.visitNode(node));
Expand Down Expand Up @@ -68,12 +67,14 @@ export class DocumentImportVisitor {
this.importsMap.set(sourceFile, imports);
}

private getDocumentElement(node: ts.NamedImports): ts.ImportSpecifier | undefined {
private getDocumentElement(node: ts.NamedImports): ts.ImportSpecifier|undefined {
const elements = node.elements;
return elements.find(el => (el.propertyName || el.name).escapedText === DOCUMENT_TOKEN_NAME);
}

private getReplaceText(documentElement: ts.ImportSpecifier): string {
return documentElement.propertyName ? `${DOCUMENT_TOKEN_NAME} as ${documentElement.name.escapedText}` : DOCUMENT_TOKEN_NAME;
return documentElement.propertyName ?
`${DOCUMENT_TOKEN_NAME} as ${documentElement.name.escapedText}` :
DOCUMENT_TOKEN_NAME;
}
}
29 changes: 18 additions & 11 deletions packages/core/schematics/migrations/move-document/google3/moveDocumentRule.ts
View file
Expand Up @@ -8,17 +8,21 @@

import {Replacement, RuleFailure, Rules} from 'tslint';
import * as ts from 'typescript';
import {addToImport, removeFromImport} from '../move-import';

import {DOCUMENT_TOKEN_NAME, DocumentImportVisitor} from '../document_import_visitor';
import {addToImport, removeFromImport} from '../move-import';


/**
* Rule that moves the DOCUMENT InjectionToken from the deprecation source in angular/platform-browser
* to the new source in angular/common. The rule also provides TSLint automatic replacements that can
* Rule that moves the DOCUMENT InjectionToken from the deprecation source in
* angular/platform-browser
* to the new source in angular/common. The rule also provides TSLint automatic replacements that
* can
* be applied in order to automatically migrate to the new source.
*/
export class Rule extends Rules.TypedRule {

static FAILURE: string = `DOCUMENT is no longer exported from @angular/platform-browser in v8. Please
static FAILURE: string =
`DOCUMENT is no longer exported from @angular/platform-browser in v8. Please
import from @angular/common`;

applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] {
Expand Down Expand Up @@ -46,16 +50,19 @@ export class Rule extends Rules.TypedRule {
// Replace the imports with the updated sources.
const platformBrowserDeclaration = platformBrowserImport.parent.parent;
const newPlatformBrowserText = removeFromImport(platformBrowserImport, DOCUMENT_TOKEN_NAME);
const newCommonText = commonImport ? addToImport(commonImport, DOCUMENT_TOKEN_NAME) : NEW_COMMON_TEXT;
const fixPlatformBrowser = new Replacement(platformBrowserDeclaration.getStart(),
platformBrowserDeclaration.getWidth(), newPlatformBrowserText);
const newCommonText =
commonImport ? addToImport(commonImport, DOCUMENT_TOKEN_NAME) : NEW_COMMON_TEXT;
const fixPlatformBrowser = new Replacement(
platformBrowserDeclaration.getStart(), platformBrowserDeclaration.getWidth(),
newPlatformBrowserText);
const fixCommon = new Replacement(platformBrowserDeclaration.end, 0, newCommonText);
const fixes: Replacement[] = fixPlatformBrowser.start > fixCommon.start ?
[fixCommon, fixPlatformBrowser] : [fixPlatformBrowser, fixCommon];
[fixCommon, fixPlatformBrowser] :
[fixPlatformBrowser, fixCommon];

failures.push(new RuleFailure(
sourceFile, documentElement.getStart(), documentElement.getWidth(), Rule.FAILURE, this.ruleName,
fixes));
sourceFile, documentElement.getStart(), documentElement.getWidth(), Rule.FAILURE,
this.ruleName, fixes));

return failures;
}
Expand Down
6 changes: 3 additions & 3 deletions packages/core/schematics/migrations/move-document/index.ts
View file
Expand Up @@ -23,8 +23,7 @@ export default function(): Rule {
const basePath = process.cwd();

if (!projectTsConfigPaths.length) {
throw new SchematicsException(
`Could not find any tsconfig file. Cannot migrate DOCUMENT
throw new SchematicsException(`Could not find any tsconfig file. Cannot migrate DOCUMENT
to new import source.`);
}

Expand Down Expand Up @@ -75,7 +74,8 @@ function runMoveDocumentMigration(tree: Tree, tsconfigPath: string, basePath: st

const platformBrowserDeclaration = platformBrowserImport.parent.parent;
const newPlatformBrowserText = removeFromImport(platformBrowserImport, DOCUMENT_TOKEN_NAME);
const newCommonText = commonImport ? addToImport(commonImport, DOCUMENT_TOKEN_NAME) : NEW_COMMON_TEXT;
const newCommonText =
commonImport ? addToImport(commonImport, DOCUMENT_TOKEN_NAME) : NEW_COMMON_TEXT;

// Replace the existing query decorator call expression with the updated
// call expression node.
Expand Down
13 changes: 7 additions & 6 deletions packages/core/schematics/migrations/move-document/move-import.ts
View file
Expand Up @@ -10,19 +10,20 @@ import * as ts from 'typescript';
export function removeFromImport(importNode: ts.NamedImports, ...keys: string[]): string {
const elements = importNode.elements;
const getName = (el: ts.ImportSpecifier) => el.propertyName ?
`${el.propertyName.escapedText} as ${el.name.escapedText}` : String(el.name.escapedText);
const elementsMap = elements
.map(getName)
.filter(el => keys.indexOf(el) === -1);
`${el.propertyName.escapedText} as ${el.name.escapedText}` :
String(el.name.escapedText);
const elementsMap = elements.map(getName).filter(el => keys.indexOf(el) === -1);
const importDeclaration = importNode.parent.parent;
return elementsMap.length > 0 ?
`import { ${elementsMap.join(', ')} } from ${importDeclaration.moduleSpecifier.getText()};` : '';
`import { ${elementsMap.join(', ')} } from ${importDeclaration.moduleSpecifier.getText()};` :
'';
}

export function addToImport(importNode: ts.NamedImports, ...keys: string[]): string {
const elements = importNode.elements;
const getName = (el: ts.ImportSpecifier) => el.propertyName ?
`${el.propertyName.escapedText} as ${el.name.escapedText}` : el.name.escapedText + '';
`${el.propertyName.escapedText} as ${el.name.escapedText}` :
el.name.escapedText + '';
const elementsMap = elements.map(getName);
elementsMap.push(...keys);
const importDeclaration = importNode.parent.parent;
Expand Down
14 changes: 8 additions & 6 deletions packages/core/schematics/test/google3/move_document_rule_spec.ts
View file
Expand Up @@ -138,15 +138,17 @@ describe('Google3 moveDocument TSLint rule', () => {
expectFileNotToContain('index.ts', `import {DOCUMENT} from '@angular/platform-browser';`);
});

it('should properly apply import replacement with existing import and leave original import', () => {
writeFile('index.ts', `
it('should properly apply import replacement with existing import and leave original import',
() => {
writeFile('index.ts', `
import {DOCUMENT, anotherImport} from '@angular/platform-browser';
import {someImport} from '@angular/common';
`);

runTSLint();
runTSLint();

expectFileToContain('index.ts', `import { someImport, DOCUMENT } from '@angular/common';`);
expectFileToContain('index.ts', `import { anotherImport } from '@angular/platform-browser';`);
});
expectFileToContain('index.ts', `import { someImport, DOCUMENT } from '@angular/common';`);
expectFileToContain(
'index.ts', `import { anotherImport } from '@angular/platform-browser';`);
});
});
15 changes: 8 additions & 7 deletions packages/core/schematics/test/move_document_migration_spec.ts
View file
Expand Up @@ -117,19 +117,20 @@ describe('move-document migration', () => {
expect(content).not.toContain(`import {DOCUMENT} from '@angular/platform-browser';`);
});

it('should properly apply import replacement with existing import and leave original import', () => {
writeFile('/index.ts', `
it('should properly apply import replacement with existing import and leave original import',
() => {
writeFile('/index.ts', `
import {DOCUMENT, anotherImport} from '@angular/platform-browser';
import {someImport} from '@angular/common';
`);

runMigration();
runMigration();

const content = tree.readContent('/index.ts');
const content = tree.readContent('/index.ts');

expect(content).toContain(`import { someImport, DOCUMENT } from '@angular/common';`);
expect(content).toContain(`import { anotherImport } from '@angular/platform-browser';`);
});
expect(content).toContain(`import { someImport, DOCUMENT } from '@angular/common';`);
expect(content).toContain(`import { anotherImport } from '@angular/platform-browser';`);
});
});

function writeFile(filePath: string, contents: string) {
Expand Down

0 comments on commit 7327824

Please sign in to comment.