Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compiler-cli): attach the correct viaModule to namespace imports #33495

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts
Expand Up @@ -1558,7 +1558,7 @@ runInEachFileSystem(() => {
const actualDeclaration = host.getDeclarationOfIdentifier(identifier);
expect(actualDeclaration).not.toBe(null);
expect(actualDeclaration !.node).toBe(expectedDeclarationNode);
expect(actualDeclaration !.viaModule).toBe(null);
expect(actualDeclaration !.viaModule).toBe('@angular/core');
});

it('should return the original declaration of an aliased class', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts
Expand Up @@ -1788,7 +1788,7 @@ runInEachFileSystem(() => {
const actualDeclaration = host.getDeclarationOfIdentifier(identifier);
expect(actualDeclaration).not.toBe(null);
expect(actualDeclaration !.node).toBe(expectedDeclarationNode);
expect(actualDeclaration !.viaModule).toBe(null);
expect(actualDeclaration !.viaModule).toBe('@angular/core');
});

it('should return the correct declaration for an inner function identifier inside an ES5 IIFE',
Expand Down
37 changes: 25 additions & 12 deletions packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts
Expand Up @@ -191,28 +191,21 @@ export class TypeScriptReflectionHost implements ReflectionHost {
return null;
}

// Ignore decorators that are defined locally (not imported).
const decl: ts.Declaration = symbol.declarations[0];
if (!ts.isImportSpecifier(decl)) {
const importDecl = getContainingImportDeclaration(decl);

// Ignore declarations that are defined locally (not imported).
if (importDecl === null) {
return null;
}

// Walk back from the specifier to find the declaration, which carries the module specifier.
const importDecl = decl.parent !.parent !.parent !;

// The module specifier is guaranteed to be a string literal, so this should always pass.
if (!ts.isStringLiteral(importDecl.moduleSpecifier)) {
// Not allowed to happen in TypeScript ASTs.
return null;
}

// Read the module specifier.
const from = importDecl.moduleSpecifier.text;

// Compute the name by which the decorator was exported, not imported.
const name = (decl.propertyName !== undefined ? decl.propertyName : decl.name).text;

return {from, name};
return {from: importDecl.moduleSpecifier.text, name: getExportedName(decl, id)};
}

/**
Expand Down Expand Up @@ -549,3 +542,23 @@ function getFarLeftIdentifier(propertyAccess: ts.PropertyAccessExpression): ts.I
}
return ts.isIdentifier(propertyAccess.expression) ? propertyAccess.expression : null;
}

/**
* Return the ImportDeclaration for the given `node` if it is either an `ImportSpecifier` or a
* `NamespaceImport`. If not return `null`.
*/
function getContainingImportDeclaration(node: ts.Node): ts.ImportDeclaration|null {
return ts.isImportSpecifier(node) ? node.parent !.parent !.parent ! :
ts.isNamespaceImport(node) ? node.parent.parent : null;
}

/**
* Compute the name by which the `decl` was exported, not imported.
* If no such declaration can be found (e.g. it is a namespace import)
* then fallback to the `originalId`.
*/
function getExportedName(decl: ts.Declaration, originalId: ts.Identifier): string {
return ts.isImportSpecifier(decl) ?
(decl.propertyName !== undefined ? decl.propertyName : decl.name).text :
originalId.text;
}
35 changes: 35 additions & 0 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Expand Up @@ -1737,6 +1737,41 @@ runInEachFileSystem(os => {
'i0.ɵɵNgModuleDefWithMeta<TestModule, never, [typeof i1.InternalRouterModule], never>');
});

it('should extract the generic type if it is provided as qualified type name from another package',
() => {
env.write(`test.ts`, `
import {NgModule} from '@angular/core';
import {RouterModule} from 'router';

@NgModule({imports: [RouterModule.forRoot()]})
export class TestModule {}`);

env.write('node_modules/router/index.d.ts', `
import {ModuleWithProviders} from '@angular/core';
import * as router2 from 'router2';

declare export class RouterModule {
static forRoot(): ModuleWithProviders<router2.Router2Module>;
}`);

env.write('node_modules/router2/index.d.ts', `
import {ɵɵNgModuleDefWithMeta} from '@angular/core';
export declare class Router2Module {
static ɵmod: ɵɵNgModuleDefWithMeta<Router2Module, never, never, never>;
}`);

env.driveMain();

const jsContents = env.getContents('test.js');
expect(jsContents).toContain('imports: [[RouterModule.forRoot()]]');

const dtsContents = env.getContents('test.d.ts');
expect(dtsContents).toContain(`import * as i1 from "router2";`);
expect(dtsContents)
.toContain(
'i0.ɵɵNgModuleDefWithMeta<TestModule, never, [typeof i1.Router2Module], never>');
});

it('should not reference a constant with a ModuleWithProviders value in module def imports',
() => {
env.write('dep.d.ts', `
Expand Down