Skip to content

Commit dc51691

Browse files
devversionmmalerba
authored andcommitted
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.
1 parent 86da318 commit dc51691

File tree

4 files changed

+24
-11
lines changed

4 files changed

+24
-11
lines changed

src/material/schematics/ng-update/test-cases/misc/material-imports.spec.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ import {join} from 'path';
55

66
describe('v8 material imports', () => {
77

8-
function writeSecondaryEntryPoint(materialPath: string, name: string, exportName = name) {
8+
function writeSecondaryEntryPoint(materialPath: string, name: string, contents: string) {
99
const entryPointPath = join(materialPath, name);
1010
mkdirpSync(entryPointPath);
11-
writeFileSync(join(entryPointPath, 'index.d.ts'), `export const ${exportName} = '';`);
11+
writeFileSync(join(entryPointPath, 'index.d.ts'), contents);
1212
}
1313

1414
it('should report imports for deleted animation constants', async () => {
@@ -22,12 +22,18 @@ describe('v8 material imports', () => {
2222
export * from './b';
2323
export * from './c';
2424
export * from './core';
25+
export * from './types';
2526
`);
2627

27-
writeSecondaryEntryPoint(materialPath, 'a');
28-
writeSecondaryEntryPoint(materialPath, 'b');
29-
writeSecondaryEntryPoint(materialPath, 'c');
30-
writeSecondaryEntryPoint(materialPath, 'core', 'VERSION');
28+
writeSecondaryEntryPoint(materialPath, 'a', `export const a = '';`);
29+
writeSecondaryEntryPoint(materialPath, 'b', `export const b = '';`);
30+
writeSecondaryEntryPoint(materialPath, 'c', `export const c = ''`);
31+
writeSecondaryEntryPoint(materialPath, 'core', `export const VERSION = '';`);
32+
writeSecondaryEntryPoint(materialPath, 'types', `
33+
export declare interface SomeInterface {
34+
event: any;
35+
}
36+
`);
3137

3238
await runFixers();
3339

src/material/schematics/ng-update/test-cases/misc/material-imports_expected_output.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,6 @@ import { /* comment */ a as a3 } from "@angular/material/a";
1313

1414
// primary entry-point export
1515
import { VERSION } from "@angular/material/core";
16+
17+
// type import
18+
import { SomeInterface } from "@angular/material/types";

src/material/schematics/ng-update/test-cases/misc/material-imports_input.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,6 @@ import {/* comment */ a as a3} from '@angular/material';
99

1010
// primary entry-point export
1111
import {VERSION} from '@angular/material';
12+
13+
// type import
14+
import {SomeInterface} from '@angular/material';

src/material/schematics/ng-update/upgrade-rules/package-imports-v8/updateAngularMaterialImportsRule.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,16 +102,17 @@ function walk(ctx: Lint.WalkContext<boolean>, checker: ts.TypeChecker): void {
102102
// would resolve to the module types provided by TypeScript itself.
103103
const symbol = getDeclarationSymbolOfNode(elementName, checker);
104104

105-
// If the symbol can't be found, add failure saying the symbol
106-
// can't be found.
107-
if (!symbol || !symbol.valueDeclaration) {
105+
// If the symbol can't be found, or no declaration could be found within
106+
// the symbol, add failure to report that the given symbol can't be found.
107+
if (!symbol || (!symbol.valueDeclaration && symbol.declarations.length === 0)) {
108108
return ctx.addFailureAtNode(element, element.getText() + Rule.SYMBOL_NOT_FOUND_FAILURE_STR);
109109
}
110110

111111
// The filename for the source file of the node that contains the
112112
// first declaration of the symbol. All symbol declarations must be
113113
// part of a defining node, so parent can be asserted to be defined.
114-
const sourceFile: string = symbol.valueDeclaration.getSourceFile().fileName;
114+
const resolvedNode: ts.Node = symbol.valueDeclaration || symbol.declarations[0];
115+
const sourceFile: string = resolvedNode.getSourceFile().fileName;
115116

116117
// File the module the symbol belongs to from a regex match of the
117118
// filename. This will always match since only "@angular/material"
@@ -166,7 +167,7 @@ function walk(ctx: Lint.WalkContext<boolean>, checker: ts.TypeChecker): void {
166167
}
167168

168169
/** Gets the symbol that contains the value declaration of the given node. */
169-
function getDeclarationSymbolOfNode(node: ts.Node, checker: ts.TypeChecker) {
170+
function getDeclarationSymbolOfNode(node: ts.Node, checker: ts.TypeChecker): ts.Symbol|undefined {
170171
const symbol = checker.getSymbolAtLocation(node);
171172

172173
// Symbols can be aliases of the declaration symbol. e.g. in named import specifiers.

0 commit comments

Comments
 (0)