Skip to content

Commit ae36486

Browse files
petebacondarwinatscott
authored andcommitted
fix(ngcc): do not scan import expressions in d.ts files (angular#37503)
It is quite common for the TS compiler to have to add synthetic types to function signatures, where the developer has not explicitly provided them. This results in `import(...)` expressions appearing in typings files. For example in `@ngrx/data` there is a class with a getter that has an implicit type: ```ts export declare class EntityCollectionServiceBase<...> { ... get store() { return this.dispatcher.store; } ... } ``` In the d.ts file for this we get: ```ts get store(): Store<import("@ngrx/data").EntityCache>; ``` Given that this file is within the `@ngrx/data` package already, this caused ngcc to believe that there was a circular dependency, causing it to fail to process the package - and in fact crash! This commit resolves this problem by ignoring `import()` expressions when scanning typings programs for dependencies. This ability was only introduced very recently in a 10.0.0 RC release, and so it has limited benefit given that up till now ngcc has been able to process libraries effectively without it. Moreover, in the rare case that a package does have such a dependency, it should get picked up by the sync ngcc+CLI integration point. PR Close angular#37503
1 parent cc552dd commit ae36486

File tree

3 files changed

+30
-3
lines changed

3 files changed

+30
-3
lines changed

packages/compiler-cli/ngcc/src/dependencies/dts_dependency_host.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ import {ModuleResolver} from './module_resolver';
1616
export class DtsDependencyHost extends EsmDependencyHost {
1717
constructor(fs: FileSystem, pathMappings?: PathMappings) {
1818
super(
19-
fs, new ModuleResolver(fs, pathMappings, ['', '.d.ts', '/index.d.ts', '.js', '/index.js']));
19+
fs, new ModuleResolver(fs, pathMappings, ['', '.d.ts', '/index.d.ts', '.js', '/index.js']),
20+
false);
2021
}
2122

2223
/**

packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,18 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88
import * as ts from 'typescript';
9-
import {AbsoluteFsPath} from '../../../src/ngtsc/file_system';
9+
import {AbsoluteFsPath, FileSystem} from '../../../src/ngtsc/file_system';
1010
import {DependencyHostBase} from './dependency_host';
11+
import {ModuleResolver} from './module_resolver';
1112

1213
/**
1314
* Helper functions for computing dependencies.
1415
*/
1516
export class EsmDependencyHost extends DependencyHostBase {
17+
constructor(
18+
fs: FileSystem, moduleResolver: ModuleResolver, private scanImportExpressions = true) {
19+
super(fs, moduleResolver);
20+
}
1621
// By skipping trivia here we don't have to account for it in the processing below
1722
// It has no relevance to capturing imports.
1823
private scanner = ts.createScanner(ts.ScriptTarget.Latest, /* skipTrivia */ true);
@@ -144,7 +149,7 @@ export class EsmDependencyHost extends DependencyHostBase {
144149

145150
// Check for dynamic import expression
146151
if (kind === ts.SyntaxKind.OpenParenToken) {
147-
return this.tryStringLiteral();
152+
return this.scanImportExpressions ? this.tryStringLiteral() : null;
148153
}
149154

150155
// Check for defaultBinding

packages/compiler-cli/ngcc/test/dependencies/dts_dependency_host_spec.ts

+21
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ runInEachFileSystem(() => {
4444
expect(dependencies.has(_('/node_modules/lib-1/sub-1'))).toBe(true);
4545
});
4646

47+
it('should ignore synthetic type imports', () => {
48+
const {dependencies, missing, deepImports} = createDependencyInfo();
49+
host.collectDependencies(
50+
_('/external/synthetic-type-imports/index.d.ts'), {dependencies, missing, deepImports});
51+
expect(dependencies.size).toBe(0);
52+
expect(missing.size).toBe(0);
53+
expect(deepImports.size).toBe(0);
54+
});
55+
4756
it('should resolve all the external re-exports of the source file', () => {
4857
const {dependencies, missing, deepImports} = createDependencyInfo();
4958
host.collectDependencies(
@@ -165,6 +174,18 @@ runInEachFileSystem(() => {
165174
},
166175
{name: _('/external/imports/package.json'), contents: '{"esm2015": "./index.js"}'},
167176
{name: _('/external/imports/index.metadata.json'), contents: 'MOCK METADATA'},
177+
{
178+
name: _('/external/synthetic-type-imports/index.d.ts'),
179+
contents: `const function foo(): Array<import("lib-1").X>;`
180+
},
181+
{
182+
name: _('/external/synthetic-type-imports/package.json'),
183+
contents: '{"esm2015": "./index.js"}'
184+
},
185+
{
186+
name: _('/external/synthetic-type-imports/index.metadata.json'),
187+
contents: 'MOCK METADATA'
188+
},
168189
{
169190
name: _('/external/re-exports/index.d.ts'),
170191
contents: `export {X} from 'lib-1';\nexport {Y} from 'lib-1/sub-1';`

0 commit comments

Comments
 (0)