honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ngcc): avoid error due to circular dependency in `EsmDependencyHost` (#34512) 

Previously, there was circular dependency between `ngcc/src/utils.ts`,
`ngcc/src/dependencies/dependency_host.ts` and
`ngcc/src/dependencies/esm_dependency_host.ts`. More specifically,
`utils.ts` would [import from `esm_dependency_host.ts`][1], which would
[import from `dependency_host.ts`][2], which would in turn
[import from `utils.ts`][3].

This might be fine in some environments/module formats, but it can cause
unclear errors in the transpiled CommonJS/UMD format (given how Node.js
handles [cycles in module resolution][4]).
(An example error can be found [here][5].)

This commit fixes the problem by moving the code that depends on
`EsmDependencyHost` out of `utils.ts` and into a dedicated file under
`dependencies/`. It also converts the `createDtsDependencyHost()`
function to a class for consistency with the rest of the
`DependencyHost`s.

[1]: https://github.com/angular/angular/blob/18d89c9c8/packages/compiler-cli/ngcc/src/utils.ts#L10
[2]: https://github.com/angular/angular/blob/18d89c9c8/packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts#L10
[3]: https://github.com/angular/angular/blob/18d89c9c8/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts#L9
[4]: https://nodejs.org/api/modules.html#modules_cycles
[5]: https://circleci.com/gh/angular/angular/577581

PR Close #34512
This commit is contained in:
George Kalpakas 2020-01-08 19:10:25 +02:00 committed by Alex Rickabaugh
parent c38195f59e
commit 07ea6cf582
7 changed files with 39 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
packages/compiler-cli/ngcc/src/dependencies/dts_dependency_host.ts Normal file
View File

@ -0,0 +1,20 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {FileSystem} from '../../../src/ngtsc/file_system';
import {PathMappings} from '../utils';
import {EsmDependencyHost} from './esm_dependency_host';
import {ModuleResolver} from './module_resolver';
/**
* Helper functions for computing dependencies via typings files.
*/
export class DtsDependencyHost extends EsmDependencyHost {
constructor(fs: FileSystem, pathMappings?: PathMappings) {
super(fs, new ModuleResolver(fs, pathMappings, ['', '.d.ts', '/index.d.ts']));
}
}

5
packages/compiler-cli/ngcc/src/main.ts
View File

@ -17,6 +17,7 @@ import {AbsoluteFsPath, FileSystem, absoluteFrom, dirname, getFileSystem, resolv
import {CommonJsDependencyHost} from './dependencies/commonjs_dependency_host'; import {CommonJsDependencyHost} from './dependencies/commonjs_dependency_host';
import {DependencyResolver, InvalidEntryPoint, PartiallyOrderedEntryPoints, SortedEntryPointsInfo} from './dependencies/dependency_resolver'; import {DependencyResolver, InvalidEntryPoint, PartiallyOrderedEntryPoints, SortedEntryPointsInfo} from './dependencies/dependency_resolver';
import {DtsDependencyHost} from './dependencies/dts_dependency_host';
import {EsmDependencyHost} from './dependencies/esm_dependency_host'; import {EsmDependencyHost} from './dependencies/esm_dependency_host';
import {ModuleResolver} from './dependencies/module_resolver'; import {ModuleResolver} from './dependencies/module_resolver';
import {UmdDependencyHost} from './dependencies/umd_dependency_host'; import {UmdDependencyHost} from './dependencies/umd_dependency_host';
@ -35,7 +36,7 @@ import {NgccConfiguration} from './packages/configuration';
import {EntryPoint, EntryPointJsonProperty, EntryPointPackageJson, SUPPORTED_FORMAT_PROPERTIES, getEntryPointFormat} from './packages/entry_point'; import {EntryPoint, EntryPointJsonProperty, EntryPointPackageJson, SUPPORTED_FORMAT_PROPERTIES, getEntryPointFormat} from './packages/entry_point';
import {makeEntryPointBundle} from './packages/entry_point_bundle'; import {makeEntryPointBundle} from './packages/entry_point_bundle';
import {Transformer} from './packages/transformer'; import {Transformer} from './packages/transformer';
import {PathMappings, createDtsDependencyHost} from './utils'; import {PathMappings} from './utils';
import {FileWriter} from './writing/file_writer'; import {FileWriter} from './writing/file_writer';
import {InPlaceFileWriter} from './writing/in_place_file_writer'; import {InPlaceFileWriter} from './writing/in_place_file_writer';
import {NewEntryPointFileWriter} from './writing/new_entry_point_file_writer'; import {NewEntryPointFileWriter} from './writing/new_entry_point_file_writer';
@ -164,7 +165,7 @@ export function mainNgcc(
const esmDependencyHost = new EsmDependencyHost(fileSystem, moduleResolver); const esmDependencyHost = new EsmDependencyHost(fileSystem, moduleResolver);
const umdDependencyHost = new UmdDependencyHost(fileSystem, moduleResolver); const umdDependencyHost = new UmdDependencyHost(fileSystem, moduleResolver);
const commonJsDependencyHost = new CommonJsDependencyHost(fileSystem, moduleResolver); const commonJsDependencyHost = new CommonJsDependencyHost(fileSystem, moduleResolver);
const dtsDependencyHost = createDtsDependencyHost(fileSystem, pathMappings); const dtsDependencyHost = new DtsDependencyHost(fileSystem, pathMappings);
const dependencyResolver = new DependencyResolver( const dependencyResolver = new DependencyResolver(
fileSystem, logger, { fileSystem, logger, {
esm5: esmDependencyHost, esm5: esmDependencyHost,

7
packages/compiler-cli/ngcc/src/utils.ts
View File

@ -7,8 +7,6 @@
*/ */
import * as ts from 'typescript'; import * as ts from 'typescript';
import {AbsoluteFsPath, FileSystem, absoluteFrom} from '../../src/ngtsc/file_system'; import {AbsoluteFsPath, FileSystem, absoluteFrom} from '../../src/ngtsc/file_system';
import {EsmDependencyHost} from './dependencies/esm_dependency_host';
import {ModuleResolver} from './dependencies/module_resolver';
/** /**
* A list (`Array`) of partially ordered `T` items. * A list (`Array`) of partially ordered `T` items.
@ -148,8 +146,3 @@ export function stripDollarSuffix(value: string): string {
export function stripExtension(fileName: string): string { export function stripExtension(fileName: string): string {
return fileName.replace(/\..+$/, ''); return fileName.replace(/\..+$/, '');
} }
export function createDtsDependencyHost(fileSystem: FileSystem, pathMappings?: PathMappings) {
return new EsmDependencyHost(
fileSystem, new ModuleResolver(fileSystem, pathMappings, ['', '.d.ts', '/index.d.ts']));
}

6
packages/compiler-cli/ngcc/test/dependencies/dependency_resolver_spec.ts
View File

@ -12,10 +12,10 @@ import {FileSystem, absoluteFrom, getFileSystem, relativeFrom} from '../../../sr
import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
import {DependencyInfo} from '../../src/dependencies/dependency_host'; import {DependencyInfo} from '../../src/dependencies/dependency_host';
import {DependencyResolver, SortedEntryPointsInfo} from '../../src/dependencies/dependency_resolver'; import {DependencyResolver, SortedEntryPointsInfo} from '../../src/dependencies/dependency_resolver';
import {DtsDependencyHost} from '../../src/dependencies/dts_dependency_host';
import {EsmDependencyHost} from '../../src/dependencies/esm_dependency_host'; import {EsmDependencyHost} from '../../src/dependencies/esm_dependency_host';
import {ModuleResolver} from '../../src/dependencies/module_resolver'; import {ModuleResolver} from '../../src/dependencies/module_resolver';
import {EntryPoint} from '../../src/packages/entry_point'; import {EntryPoint} from '../../src/packages/entry_point';
import {createDtsDependencyHost} from '../../src/utils';
import {MockLogger} from '../helpers/mock_logger'; import {MockLogger} from '../helpers/mock_logger';
@ -37,7 +37,7 @@ runInEachFileSystem(() => {
fs = getFileSystem(); fs = getFileSystem();
moduleResolver = new ModuleResolver(fs); moduleResolver = new ModuleResolver(fs);
host = new EsmDependencyHost(fs, moduleResolver); host = new EsmDependencyHost(fs, moduleResolver);
dtsHost = createDtsDependencyHost(fs); dtsHost = new DtsDependencyHost(fs);
resolver = new DependencyResolver(fs, new MockLogger(), {esm5: host, esm2015: host}, dtsHost); resolver = new DependencyResolver(fs, new MockLogger(), {esm5: host, esm2015: host}, dtsHost);
}); });
@ -324,7 +324,7 @@ runInEachFileSystem(() => {
it('should use the appropriate DependencyHost for each entry-point', () => { it('should use the appropriate DependencyHost for each entry-point', () => {
const esm5Host = new EsmDependencyHost(fs, moduleResolver); const esm5Host = new EsmDependencyHost(fs, moduleResolver);
const esm2015Host = new EsmDependencyHost(fs, moduleResolver); const esm2015Host = new EsmDependencyHost(fs, moduleResolver);
const dtsHost = createDtsDependencyHost(fs); const dtsHost = new DtsDependencyHost(fs);
resolver = new DependencyResolver( resolver = new DependencyResolver(
fs, new MockLogger(), {esm5: esm5Host, esm2015: esm2015Host}, dtsHost); fs, new MockLogger(), {esm5: esm5Host, esm2015: esm2015Host}, dtsHost);
spyOn(esm5Host, 'collectDependencies') spyOn(esm5Host, 'collectDependencies')

4
packages/compiler-cli/ngcc/test/dependencies/esm_dependency_host_spec.ts
View File

@ -11,9 +11,9 @@ import {absoluteFrom, getFileSystem, relativeFrom} from '../../../src/ngtsc/file
import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
import {loadTestFiles} from '../../../test/helpers'; import {loadTestFiles} from '../../../test/helpers';
import {createDependencyInfo} from '../../src/dependencies/dependency_host'; import {createDependencyInfo} from '../../src/dependencies/dependency_host';
import {DtsDependencyHost} from '../../src/dependencies/dts_dependency_host';
import {EsmDependencyHost, hasImportOrReexportStatements, isStringImportOrReexport} from '../../src/dependencies/esm_dependency_host'; import {EsmDependencyHost, hasImportOrReexportStatements, isStringImportOrReexport} from '../../src/dependencies/esm_dependency_host';
import {ModuleResolver} from '../../src/dependencies/module_resolver'; import {ModuleResolver} from '../../src/dependencies/module_resolver';
import {createDtsDependencyHost} from '../../src/utils';
runInEachFileSystem(() => { runInEachFileSystem(() => {
@ -160,7 +160,7 @@ runInEachFileSystem(() => {
expect(jsDeps.missing.has(relativeFrom('./internal-typings'))).toBeTruthy(); expect(jsDeps.missing.has(relativeFrom('./internal-typings'))).toBeTruthy();
// Typings mode will pick up `internal-typings.d.ts` dependency // Typings mode will pick up `internal-typings.d.ts` dependency
const dtsHost = createDtsDependencyHost(fs); const dtsHost = new DtsDependencyHost(fs);
const dtsDeps = createDependencyInfo(); const dtsDeps = createDependencyInfo();
dtsHost.collectDependencies(_('/external/index.d.ts'), dtsDeps); dtsHost.collectDependencies(_('/external/index.d.ts'), dtsDeps);
expect(dtsDeps.dependencies.size).toEqual(2); expect(dtsDeps.dependencies.size).toEqual(2);

11
packages/compiler-cli/ngcc/test/entry_point_finder/directory_walker_entry_point_finder_spec.ts
View File

@ -9,12 +9,13 @@ import {AbsoluteFsPath, FileSystem, absoluteFrom, getFileSystem, relative} from
import {TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
import {loadTestFiles} from '../../../test/helpers'; import {loadTestFiles} from '../../../test/helpers';
import {DependencyResolver} from '../../src/dependencies/dependency_resolver'; import {DependencyResolver} from '../../src/dependencies/dependency_resolver';
import {DtsDependencyHost} from '../../src/dependencies/dts_dependency_host';
import {EsmDependencyHost} from '../../src/dependencies/esm_dependency_host'; import {EsmDependencyHost} from '../../src/dependencies/esm_dependency_host';
import {ModuleResolver} from '../../src/dependencies/module_resolver'; import {ModuleResolver} from '../../src/dependencies/module_resolver';
import {DirectoryWalkerEntryPointFinder} from '../../src/entry_point_finder/directory_walker_entry_point_finder'; import {DirectoryWalkerEntryPointFinder} from '../../src/entry_point_finder/directory_walker_entry_point_finder';
import {NgccConfiguration} from '../../src/packages/configuration'; import {NgccConfiguration} from '../../src/packages/configuration';
import {EntryPoint} from '../../src/packages/entry_point'; import {EntryPoint} from '../../src/packages/entry_point';
import {PathMappings, createDtsDependencyHost} from '../../src/utils'; import {PathMappings} from '../../src/utils';
import {MockLogger} from '../helpers/mock_logger'; import {MockLogger} from '../helpers/mock_logger';
runInEachFileSystem(() => { runInEachFileSystem(() => {
@ -30,7 +31,7 @@ runInEachFileSystem(() => {
_Abs = absoluteFrom; _Abs = absoluteFrom;
logger = new MockLogger(); logger = new MockLogger();
const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs)); const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs));
const dtsHost = createDtsDependencyHost(fs); const dtsHost = new DtsDependencyHost(fs);
resolver = new DependencyResolver(fs, logger, {esm2015: srcHost}, dtsHost); resolver = new DependencyResolver(fs, logger, {esm2015: srcHost}, dtsHost);
config = new NgccConfiguration(fs, _Abs('/')); config = new NgccConfiguration(fs, _Abs('/'));
}); });
@ -154,7 +155,7 @@ runInEachFileSystem(() => {
...createPackage(_Abs('/path_mapped/dist/lib/pkg3'), 'test'), ...createPackage(_Abs('/path_mapped/dist/lib/pkg3'), 'test'),
]); ]);
const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs, pathMappings)); const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs, pathMappings));
const dtsHost = createDtsDependencyHost(fs, pathMappings); const dtsHost = new DtsDependencyHost(fs, pathMappings);
resolver = new DependencyResolver(fs, logger, {esm2015: srcHost}, dtsHost); resolver = new DependencyResolver(fs, logger, {esm2015: srcHost}, dtsHost);
const finder = new DirectoryWalkerEntryPointFinder( const finder = new DirectoryWalkerEntryPointFinder(
fs, config, logger, resolver, basePath, pathMappings); fs, config, logger, resolver, basePath, pathMappings);
@ -182,7 +183,7 @@ runInEachFileSystem(() => {
...createPackage(_Abs('/path_mapped/dist'), 'pkg2'), ...createPackage(_Abs('/path_mapped/dist'), 'pkg2'),
]); ]);
const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs, pathMappings)); const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs, pathMappings));
const dtsHost = createDtsDependencyHost(fs, pathMappings); const dtsHost = new DtsDependencyHost(fs, pathMappings);
resolver = new DependencyResolver(fs, logger, {esm2015: srcHost}, dtsHost); resolver = new DependencyResolver(fs, logger, {esm2015: srcHost}, dtsHost);
const finder = new DirectoryWalkerEntryPointFinder( const finder = new DirectoryWalkerEntryPointFinder(
fs, config, logger, resolver, basePath, pathMappings); fs, config, logger, resolver, basePath, pathMappings);
@ -220,4 +221,4 @@ runInEachFileSystem(() => {
} }
}); });
}); });
}); });

9
packages/compiler-cli/ngcc/test/entry_point_finder/targeted_entry_point_finder_spec.ts
View File

@ -9,12 +9,13 @@ import {AbsoluteFsPath, FileSystem, absoluteFrom, getFileSystem, relative} from
import {TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
import {loadTestFiles} from '../../../test/helpers'; import {loadTestFiles} from '../../../test/helpers';
import {DependencyResolver} from '../../src/dependencies/dependency_resolver'; import {DependencyResolver} from '../../src/dependencies/dependency_resolver';
import {DtsDependencyHost} from '../../src/dependencies/dts_dependency_host';
import {EsmDependencyHost} from '../../src/dependencies/esm_dependency_host'; import {EsmDependencyHost} from '../../src/dependencies/esm_dependency_host';
import {ModuleResolver} from '../../src/dependencies/module_resolver'; import {ModuleResolver} from '../../src/dependencies/module_resolver';
import {TargetedEntryPointFinder} from '../../src/entry_point_finder/targeted_entry_point_finder'; import {TargetedEntryPointFinder} from '../../src/entry_point_finder/targeted_entry_point_finder';
import {NgccConfiguration} from '../../src/packages/configuration'; import {NgccConfiguration} from '../../src/packages/configuration';
import {EntryPoint} from '../../src/packages/entry_point'; import {EntryPoint} from '../../src/packages/entry_point';
import {PathMappings, createDtsDependencyHost} from '../../src/utils'; import {PathMappings} from '../../src/utils';
import {MockLogger} from '../helpers/mock_logger'; import {MockLogger} from '../helpers/mock_logger';
runInEachFileSystem(() => { runInEachFileSystem(() => {
@ -30,7 +31,7 @@ runInEachFileSystem(() => {
_Abs = absoluteFrom; _Abs = absoluteFrom;
logger = new MockLogger(); logger = new MockLogger();
const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs)); const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs));
const dtsHost = createDtsDependencyHost(fs); const dtsHost = new DtsDependencyHost(fs);
resolver = new DependencyResolver(fs, logger, {esm2015: srcHost}, dtsHost); resolver = new DependencyResolver(fs, logger, {esm2015: srcHost}, dtsHost);
config = new NgccConfiguration(fs, _Abs('/')); config = new NgccConfiguration(fs, _Abs('/'));
}); });
@ -186,7 +187,7 @@ runInEachFileSystem(() => {
...createPackage(_Abs('/path_mapped/dist'), 'pkg5'), ...createPackage(_Abs('/path_mapped/dist'), 'pkg5'),
]); ]);
const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs, pathMappings)); const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs, pathMappings));
const dtsHost = createDtsDependencyHost(fs, pathMappings); const dtsHost = new DtsDependencyHost(fs, pathMappings);
resolver = new DependencyResolver(fs, logger, {esm2015: srcHost}, dtsHost); resolver = new DependencyResolver(fs, logger, {esm2015: srcHost}, dtsHost);
const finder = new TargetedEntryPointFinder( const finder = new TargetedEntryPointFinder(
fs, config, logger, resolver, basePath, targetPath, pathMappings); fs, config, logger, resolver, basePath, targetPath, pathMappings);
@ -216,7 +217,7 @@ runInEachFileSystem(() => {
...createPackage(_Abs('/path_mapped/dist'), 'pkg2'), ...createPackage(_Abs('/path_mapped/dist'), 'pkg2'),
]); ]);
const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs, pathMappings)); const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs, pathMappings));
const dtsHost = createDtsDependencyHost(fs, pathMappings); const dtsHost = new DtsDependencyHost(fs, pathMappings);
resolver = new DependencyResolver(fs, logger, {esm2015: srcHost}, dtsHost); resolver = new DependencyResolver(fs, logger, {esm2015: srcHost}, dtsHost);
const finder = new TargetedEntryPointFinder( const finder = new TargetedEntryPointFinder(
fs, config, logger, resolver, basePath, targetPath, pathMappings); fs, config, logger, resolver, basePath, targetPath, pathMappings);