From dc5169154b912e780d03e18846bf1426e011a6b5 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 24 May 2019 18:22:48 +0200 Subject: [PATCH] fix(ng-update): type imports not migrated to secondary entry-points (#16108) The V8 import migration does not seem to properly handle the migration of type imports as these don't have an actual Symbol `value` declaration but rather just a plain declaration. We need to support both (value declaration for real values) and fall back to the first non-value declaration for types. --- .../test-cases/misc/material-imports.spec.ts | 18 ++++++++++++------ .../misc/material-imports_expected_output.ts | 3 +++ .../test-cases/misc/material-imports_input.ts | 3 +++ .../updateAngularMaterialImportsRule.ts | 11 ++++++----- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/material/schematics/ng-update/test-cases/misc/material-imports.spec.ts b/src/material/schematics/ng-update/test-cases/misc/material-imports.spec.ts index 220427cbca2..bb62bca51b4 100644 --- a/src/material/schematics/ng-update/test-cases/misc/material-imports.spec.ts +++ b/src/material/schematics/ng-update/test-cases/misc/material-imports.spec.ts @@ -5,10 +5,10 @@ import {join} from 'path'; describe('v8 material imports', () => { - function writeSecondaryEntryPoint(materialPath: string, name: string, exportName = name) { + function writeSecondaryEntryPoint(materialPath: string, name: string, contents: string) { const entryPointPath = join(materialPath, name); mkdirpSync(entryPointPath); - writeFileSync(join(entryPointPath, 'index.d.ts'), `export const ${exportName} = '';`); + writeFileSync(join(entryPointPath, 'index.d.ts'), contents); } it('should report imports for deleted animation constants', async () => { @@ -22,12 +22,18 @@ describe('v8 material imports', () => { export * from './b'; export * from './c'; export * from './core'; + export * from './types'; `); - writeSecondaryEntryPoint(materialPath, 'a'); - writeSecondaryEntryPoint(materialPath, 'b'); - writeSecondaryEntryPoint(materialPath, 'c'); - writeSecondaryEntryPoint(materialPath, 'core', 'VERSION'); + writeSecondaryEntryPoint(materialPath, 'a', `export const a = '';`); + writeSecondaryEntryPoint(materialPath, 'b', `export const b = '';`); + writeSecondaryEntryPoint(materialPath, 'c', `export const c = ''`); + writeSecondaryEntryPoint(materialPath, 'core', `export const VERSION = '';`); + writeSecondaryEntryPoint(materialPath, 'types', ` + export declare interface SomeInterface { + event: any; + } + `); await runFixers(); diff --git a/src/material/schematics/ng-update/test-cases/misc/material-imports_expected_output.ts b/src/material/schematics/ng-update/test-cases/misc/material-imports_expected_output.ts index 705eefe952e..a57a1f55433 100644 --- a/src/material/schematics/ng-update/test-cases/misc/material-imports_expected_output.ts +++ b/src/material/schematics/ng-update/test-cases/misc/material-imports_expected_output.ts @@ -13,3 +13,6 @@ import { /* comment */ a as a3 } from "@angular/material/a"; // primary entry-point export import { VERSION } from "@angular/material/core"; + +// type import +import { SomeInterface } from "@angular/material/types"; diff --git a/src/material/schematics/ng-update/test-cases/misc/material-imports_input.ts b/src/material/schematics/ng-update/test-cases/misc/material-imports_input.ts index da2638d0075..0cc2db12b5a 100644 --- a/src/material/schematics/ng-update/test-cases/misc/material-imports_input.ts +++ b/src/material/schematics/ng-update/test-cases/misc/material-imports_input.ts @@ -9,3 +9,6 @@ import {/* comment */ a as a3} from '@angular/material'; // primary entry-point export import {VERSION} from '@angular/material'; + +// type import +import {SomeInterface} from '@angular/material'; diff --git a/src/material/schematics/ng-update/upgrade-rules/package-imports-v8/updateAngularMaterialImportsRule.ts b/src/material/schematics/ng-update/upgrade-rules/package-imports-v8/updateAngularMaterialImportsRule.ts index f9f82adb98a..b0f85736360 100644 --- a/src/material/schematics/ng-update/upgrade-rules/package-imports-v8/updateAngularMaterialImportsRule.ts +++ b/src/material/schematics/ng-update/upgrade-rules/package-imports-v8/updateAngularMaterialImportsRule.ts @@ -102,16 +102,17 @@ function walk(ctx: Lint.WalkContext, checker: ts.TypeChecker): void { // would resolve to the module types provided by TypeScript itself. const symbol = getDeclarationSymbolOfNode(elementName, checker); - // If the symbol can't be found, add failure saying the symbol - // can't be found. - if (!symbol || !symbol.valueDeclaration) { + // If the symbol can't be found, or no declaration could be found within + // the symbol, add failure to report that the given symbol can't be found. + if (!symbol || (!symbol.valueDeclaration && symbol.declarations.length === 0)) { return ctx.addFailureAtNode(element, element.getText() + Rule.SYMBOL_NOT_FOUND_FAILURE_STR); } // The filename for the source file of the node that contains the // first declaration of the symbol. All symbol declarations must be // part of a defining node, so parent can be asserted to be defined. - const sourceFile: string = symbol.valueDeclaration.getSourceFile().fileName; + const resolvedNode: ts.Node = symbol.valueDeclaration || symbol.declarations[0]; + const sourceFile: string = resolvedNode.getSourceFile().fileName; // File the module the symbol belongs to from a regex match of the // filename. This will always match since only "@angular/material" @@ -166,7 +167,7 @@ function walk(ctx: Lint.WalkContext, checker: ts.TypeChecker): void { } /** Gets the symbol that contains the value declaration of the given node. */ -function getDeclarationSymbolOfNode(node: ts.Node, checker: ts.TypeChecker) { +function getDeclarationSymbolOfNode(node: ts.Node, checker: ts.TypeChecker): ts.Symbol|undefined { const symbol = checker.getSymbolAtLocation(node); // Symbols can be aliases of the declaration symbol. e.g. in named import specifiers.