refactor(ngcc): avoid repeated file resolution during dependency scan (#32181)

During the recursive processing of dependencies, ngcc resolves the
requested file to an actual location on disk, by testing various
extensions. For recursive calls however, the path is known to have been
resolved in the module resolver. Therefore, it is safe to move the path
resolution to the initial caller into the recursive process.

Note that this is not expected to improve the performance of ngcc, as
the call to `resolveFileWithPostfixes` is known to succeed immediately,
as the provided path is known to exist without needing to add any
postfixes. Furthermore, the FileSystem caches whether files exist, so
the additional check that we used to do was cheap.

PR Close #32181
This commit is contained in:
JoostK 2019-08-18 21:04:11 +02:00 committed by Andrew Kushnir
parent e85ec23037
commit ae142a6827
4 changed files with 22 additions and 30 deletions

View File

@ -8,7 +8,6 @@
import * as ts from 'typescript';
import {AbsoluteFsPath} from '../../../src/ngtsc/file_system';
import {isRequireCall} from '../host/commonjs_host';
import {resolveFileWithPostfixes} from '../utils';
import {DependencyHostBase} from './dependency_host';
import {ResolvedDeepImport, ResolvedRelativeModule} from './module_resolver';
@ -31,11 +30,7 @@ export class CommonJsDependencyHost extends DependencyHostBase {
protected recursivelyFindDependencies(
file: AbsoluteFsPath, dependencies: Set<AbsoluteFsPath>, missing: Set<string>,
deepImports: Set<AbsoluteFsPath>, alreadySeen: Set<AbsoluteFsPath>): void {
const resolvedFile = resolveFileWithPostfixes(this.fs, file, ['', '.js', '/index.js']);
if (resolvedFile === null) {
return;
}
const fromContents = this.fs.readFile(resolvedFile);
const fromContents = this.fs.readFile(file);
if (!this.hasRequireCalls(fromContents)) {
// Avoid parsing the source file as there are no imports.
@ -43,8 +38,8 @@ export class CommonJsDependencyHost extends DependencyHostBase {
}
// Parse the source into a TypeScript AST and then walk it looking for imports and re-exports.
const sf = ts.createSourceFile(
resolvedFile, fromContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS);
const sf =
ts.createSourceFile(file, fromContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS);
for (const statement of sf.statements) {
const declarations =
@ -52,7 +47,7 @@ export class CommonJsDependencyHost extends DependencyHostBase {
for (const declaration of declarations) {
if (declaration.initializer && isRequireCall(declaration.initializer)) {
const importPath = declaration.initializer.arguments[0].text;
const resolvedModule = this.moduleResolver.resolveModuleImport(importPath, resolvedFile);
const resolvedModule = this.moduleResolver.resolveModuleImport(importPath, file);
if (resolvedModule) {
if (resolvedModule instanceof ResolvedRelativeModule) {
const internalDependency = resolvedModule.modulePath;

View File

@ -6,6 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/
import {AbsoluteFsPath, FileSystem, PathSegment} from '../../../src/ngtsc/file_system';
import {resolveFileWithPostfixes} from '../utils';
import {ModuleResolver} from './module_resolver';
export interface DependencyHost {
@ -32,10 +34,15 @@ export abstract class DependencyHostBase implements DependencyHost {
const dependencies = new Set<AbsoluteFsPath>();
const missing = new Set<AbsoluteFsPath|PathSegment>();
const deepImports = new Set<AbsoluteFsPath>();
const alreadySeen = new Set<AbsoluteFsPath>();
this.recursivelyFindDependencies(
entryPointPath, dependencies, missing, deepImports, alreadySeen);
const resolvedFile =
resolveFileWithPostfixes(this.fs, entryPointPath, ['', '.js', '/index.js']);
if (resolvedFile !== null) {
const alreadySeen = new Set<AbsoluteFsPath>();
this.recursivelyFindDependencies(
resolvedFile, dependencies, missing, deepImports, alreadySeen);
}
return {dependencies, missing, deepImports};
}

View File

@ -7,7 +7,6 @@
*/
import * as ts from 'typescript';
import {AbsoluteFsPath} from '../../../src/ngtsc/file_system';
import {resolveFileWithPostfixes} from '../utils';
import {DependencyHostBase} from './dependency_host';
import {ResolvedDeepImport, ResolvedRelativeModule} from './module_resolver';
@ -30,11 +29,7 @@ export class EsmDependencyHost extends DependencyHostBase {
protected recursivelyFindDependencies(
file: AbsoluteFsPath, dependencies: Set<AbsoluteFsPath>, missing: Set<string>,
deepImports: Set<string>, alreadySeen: Set<AbsoluteFsPath>): void {
const resolvedFile = resolveFileWithPostfixes(this.fs, file, ['', '.js', '/index.js']);
if (resolvedFile === null) {
return;
}
const fromContents = this.fs.readFile(resolvedFile);
const fromContents = this.fs.readFile(file);
if (!hasImportOrReexportStatements(fromContents)) {
// Avoid parsing the source file as there are no imports.
@ -42,8 +37,8 @@ export class EsmDependencyHost extends DependencyHostBase {
}
// Parse the source into a TypeScript AST and then walk it looking for imports and re-exports.
const sf = ts.createSourceFile(
resolvedFile, fromContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS);
const sf =
ts.createSourceFile(file, fromContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS);
sf.statements
// filter out statements that are not imports or reexports
.filter(isStringImportOrReexport)
@ -51,7 +46,7 @@ export class EsmDependencyHost extends DependencyHostBase {
.map(stmt => stmt.moduleSpecifier.text)
// Resolve this module id into an absolute path
.forEach(importPath => {
const resolvedModule = this.moduleResolver.resolveModuleImport(importPath, resolvedFile);
const resolvedModule = this.moduleResolver.resolveModuleImport(importPath, file);
if (resolvedModule) {
if (resolvedModule instanceof ResolvedRelativeModule) {
const internalDependency = resolvedModule.modulePath;

View File

@ -8,7 +8,6 @@
import * as ts from 'typescript';
import {AbsoluteFsPath} from '../../../src/ngtsc/file_system';
import {getImportsOfUmdModule, parseStatementForUmdModule} from '../host/umd_host';
import {resolveFileWithPostfixes} from '../utils';
import {DependencyHostBase} from './dependency_host';
import {ResolvedDeepImport, ResolvedRelativeModule} from './module_resolver';
@ -31,11 +30,7 @@ export class UmdDependencyHost extends DependencyHostBase {
protected recursivelyFindDependencies(
file: AbsoluteFsPath, dependencies: Set<AbsoluteFsPath>, missing: Set<string>,
deepImports: Set<string>, alreadySeen: Set<AbsoluteFsPath>): void {
const resolvedFile = resolveFileWithPostfixes(this.fs, file, ['', '.js', '/index.js']);
if (resolvedFile === null) {
return;
}
const fromContents = this.fs.readFile(resolvedFile);
const fromContents = this.fs.readFile(file);
if (!this.hasRequireCalls(fromContents)) {
// Avoid parsing the source file as there are no imports.
@ -43,8 +38,8 @@ export class UmdDependencyHost extends DependencyHostBase {
}
// Parse the source into a TypeScript AST and then walk it looking for imports and re-exports.
const sf = ts.createSourceFile(
resolvedFile, fromContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS);
const sf =
ts.createSourceFile(file, fromContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS);
if (sf.statements.length !== 1) {
return;
@ -57,7 +52,7 @@ export class UmdDependencyHost extends DependencyHostBase {
}
umdImports.forEach(umdImport => {
const resolvedModule = this.moduleResolver.resolveModuleImport(umdImport.path, resolvedFile);
const resolvedModule = this.moduleResolver.resolveModuleImport(umdImport.path, file);
if (resolvedModule) {
if (resolvedModule instanceof ResolvedRelativeModule) {
const internalDependency = resolvedModule.modulePath;