Skip to content

Commit

Permalink
fix(ngcc): use aliased exported types correctly
Browse files Browse the repository at this point in the history 
If a type has been renamed when it was exported, we need to
reference the external public alias name rather than the internal
original name for the type. Otherwise we will try to import the
type by its internal name, which is not publicly accessible.

Fixes #38238
  • Loading branch information
@petebacondarwin
petebacondarwin committed Sep 1, 2020
1 parent 71acf9d commit d2717e0
Show file tree
Hide file tree
Showing 12 changed files with 77 additions and 10 deletions.
5 changes: 3 additions & 2 deletions packages/compiler-cli/ngcc/src/host/commonjs_host.ts
View file
Expand Up @@ -181,7 +181,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
} else {
return {
name,
declaration: {node: null, known: null, expression, viaModule: null},
declaration: {node: null, known: null, expression, viaModule: null, importName: null},
};
}
}
Expand All @@ -204,7 +204,8 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
return null;
}
const viaModule = isExternalImport(importPath) ? importPath : null;
return {node: module, known: null, viaModule, identity: null};
const importName = viaModule && id.text;
return {node: module, known: null, viaModule, importName, identity: null};
}

private resolveModuleName(moduleName: string, containingFile: ts.SourceFile): ts.SourceFile
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/ngcc/src/host/esm2015_host.ts
View file
Expand Up @@ -1606,7 +1606,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
kind: TypeValueReferenceKind.IMPORTED,
valueDeclaration: decl.node,
moduleName: decl.viaModule,
importedName: decl.node.name.text,
importedName: decl.importName!, // viaModule and importName are both non-null together
nestedPath: null,
};
} else {
Expand Down
1 change: 1 addition & 0 deletions packages/compiler-cli/ngcc/src/host/esm5_host.ts
View file
Expand Up @@ -86,6 +86,7 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
known: nonEmittedNorImportedTsHelperDeclaration,
node: null,
viaModule: null,
importName: null,
};
}
}
Expand Down
17 changes: 11 additions & 6 deletions packages/compiler-cli/ngcc/src/host/umd_host.ts
View file
Expand Up @@ -175,11 +175,15 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
if (decl.node !== null) {
reexports.push({
name,
declaration: {node: decl.node, known: null, viaModule, identity: decl.identity}
declaration:
{node: decl.node, known: null, viaModule, importName: name, identity: decl.identity}
});
} else {
reexports.push(
{name, declaration: {node: null, known: null, expression: decl.expression, viaModule}});
reexports.push({
name,
declaration:
{node: null, known: null, expression: decl.expression, viaModule, importName: name}
});
}
});
return reexports;
Expand All @@ -203,7 +207,7 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
}
return {
name,
declaration: {node: null, known: null, expression, viaModule: null},
declaration: {node: null, known: null, expression, viaModule: null, importName: null},
};
}

Expand Down Expand Up @@ -245,8 +249,9 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
// did not know that we were importing the declaration.
const viaModule =
declaration.viaModule === null ? moduleDeclaration.viaModule : declaration.viaModule;
const importName = viaModule && id.text;

return {...declaration, viaModule, known: getTsHelperFnFromIdentifier(id)};
return {...declaration, viaModule, importName, known: getTsHelperFnFromIdentifier(id)};
}

private getUmdModuleDeclaration(id: ts.Identifier): Declaration|null {
Expand All @@ -261,7 +266,7 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
}

const viaModule = isExternalImport(importPath) ? importPath : null;
return {node: module, viaModule, known: null, identity: null};
return {node: module, viaModule, known: null, identity: null, importName: null};
}

private getImportPathFromParameter(id: ts.Identifier): string|null {
Expand Down
3 changes: 3 additions & 0 deletions packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts
View file
Expand Up @@ -1780,6 +1780,7 @@ exports.MissingClass2 = MissingClass2;
known: knownAs,
node: getHelperDeclaration(helperName),
viaModule,
importName: viaModule && helperName,
identity: null,
});
};
Expand Down Expand Up @@ -2104,6 +2105,7 @@ exports.MissingClass2 = MissingClass2;
expression: helperIdentifier,
node: null,
viaModule: null,
importName: null,
});
};

Expand Down Expand Up @@ -2138,6 +2140,7 @@ exports.MissingClass2 = MissingClass2;
expression: helperIdentifier,
node: null,
viaModule: null,
importName: null,
});
};

Expand Down
44 changes: 44 additions & 0 deletions packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts
View file
Expand Up @@ -1140,6 +1140,50 @@ runInEachFileSystem(() => {
});

describe('getConstructorParameters()', () => {
describe('getConstructorParameters()', () => {
it('should find the correct imported identifier for decorated constructor parameter types',
() => {
const files = [
{
name: _('/node_modules/shared-lib/foo.d.ts'),
contents: `
declare class Foo {}
export {Foo as Bar};
`,
},
{
name: _('/node_modules/shared-lib/index.d.ts'),
contents: `
export {Bar as Baz} from './foo';
`,
},
{
name: _('/main.js'),
contents: `
import {Baz} from 'shared-lib';
export class SomeClass {
constructor(arg1) {}
}
SomeClass.ctorParameters = [{ type: Baz }];
`,
},
];

loadTestFiles(files);
const bundle = makeTestBundleProgram(_('/main.js'));
const host =
createHost(bundle, new Esm2015ReflectionHost(new MockLogger(), false, bundle));
const classNode = getDeclaration(
bundle.program, _('/main.js'), 'SomeClass', isNamedClassDeclaration);

const parameters = host.getConstructorParameters(classNode)!;

expect(parameters).toEqual([jasmine.objectContaining({name: 'arg1'})]);
expectTypeValueReferencesForParameters(parameters, ['Baz'], 'shared-lib');
});
});

it('should find the decorated constructor parameters', () => {
loadFakeCore(getFileSystem());
loadTestFiles([SOME_DIRECTIVE_FILE]);
Expand Down
3 changes: 3 additions & 0 deletions packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts
View file
Expand Up @@ -1799,6 +1799,7 @@ runInEachFileSystem(() => {
known: knownAs,
node: getHelperDeclaration(helperName),
viaModule,
importName: viaModule && helperName,
identity: null,
});
};
Expand Down Expand Up @@ -2195,6 +2196,7 @@ runInEachFileSystem(() => {
expression: helperIdentifier,
node: null,
viaModule: null,
importName: null,
});
};

Expand Down Expand Up @@ -2226,6 +2228,7 @@ runInEachFileSystem(() => {
expression: helperIdentifier,
node: null,
viaModule: null,
importName: null,
});
};

Expand Down
5 changes: 4 additions & 1 deletion packages/compiler-cli/ngcc/test/host/umd_host_spec.ts
View file
Expand Up @@ -1591,7 +1591,7 @@ runInEachFileSystem(() => {
`;
break;
case 'inlined_with_suffix':
fileHeaderWithUmd = `
fileHeaderWithUmd = `
(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports)) :
typeof define === 'function' && define.amd ? define('test', ['exports'], factory) :
Expand Down Expand Up @@ -1904,6 +1904,7 @@ runInEachFileSystem(() => {
known: knownAs,
node: getHelperDeclaration(factoryFn, helperName),
viaModule,
importName: viaModule && helperName,
identity: null,
});
};
Expand Down Expand Up @@ -2207,6 +2208,7 @@ runInEachFileSystem(() => {
expression: helperIdentifier,
node: null,
viaModule: null,
importName: null,
});
};

Expand Down Expand Up @@ -2246,6 +2248,7 @@ runInEachFileSystem(() => {
expression: helperIdentifier,
node: null,
viaModule: null,
importName: null,
});
};

Expand Down
1 change: 1 addition & 0 deletions packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts
View file
Expand Up @@ -960,6 +960,7 @@ runInEachFileSystem(() => {
known: tsHelperFn,
node: id,
viaModule: null,
importName: null,
identity: null,
};
}
Expand Down
1 change: 1 addition & 0 deletions packages/compiler-cli/src/ngtsc/reflection/src/host.ts
View file
Expand Up @@ -540,6 +540,7 @@ export interface BaseDeclaration<T extends ts.Declaration = ts.Declaration> {
* of the application and was not imported from an absolute path, this will be `null`.
*/
viaModule: string|null;
importName: string|null;

/**
* TypeScript reference to the declaration itself, if one exists.
Expand Down
3 changes: 3 additions & 0 deletions packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts
View file
Expand Up @@ -303,6 +303,7 @@ export class TypeScriptReflectionHost implements ReflectionHost {
importInfo !== null && importInfo.from !== null && !importInfo.from.startsWith('.') ?
importInfo.from :
null;
const importName = importInfo && importInfo.name;

// Now, resolve the Symbol to its declaration by following any and all aliases.
while (symbol.flags & ts.SymbolFlags.Alias) {
Expand All @@ -316,13 +317,15 @@ export class TypeScriptReflectionHost implements ReflectionHost {
node: symbol.valueDeclaration,
known: null,
viaModule,
importName,
identity: null,
};
} else if (symbol.declarations !== undefined && symbol.declarations.length > 0) {
return {
node: symbol.declarations[0],
known: null,
viaModule,
importName,
identity: null,
};
} else {
Expand Down
2 changes: 2 additions & 0 deletions packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts
View file
Expand Up @@ -363,6 +363,7 @@ runInEachFileSystem(() => {
node: targetDecl,
known: null,
viaModule: 'absolute',
importName: 'Target',
identity: null,
});
});
Expand Down Expand Up @@ -394,6 +395,7 @@ runInEachFileSystem(() => {
node: targetDecl,
known: null,
viaModule: 'absolute',
importName: 'Target',
identity: null,
});
});
Expand Down

0 comments on commit d2717e0

Please sign in to comment.