fix(ivy): ngcc - do not analyze files outside the current package (#30591)
Our module resolution prefers `.js` files over `.d.ts` files because occasionally libraries publish their typings in the same directory structure as the compiled JS files, i.e. adjacent to each other. The standard TS module resolution would pick up the typings file and add that to the `ts.Program` and so they would be ignored by our analyzers. But we need those JS files, if they are part of the current package. But this meant that we also bring in JS files from external imports from outside the package, which is not desired. This was happening for the `@fire/storage` enty-point that was importing the `firebase/storage` path. In this commit we solve this problem, for the case of imports coming from a completely different package, by saying that any file that is outside the package root directory must be an external import and so we do not analyze those files. This does not solve the potential problem of imports between secondary entry-points within a package but so far that does not appear to be a problem. PR Close #30591
This commit is contained in:

committed by
Kara Erickson

parent
42036f4b79
commit
f690a4e0af
@ -10,7 +10,7 @@ import * as ts from 'typescript';
|
||||
|
||||
import {BaseDefDecoratorHandler, ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, PipeDecoratorHandler, ReferencesRegistry, ResourceLoader} from '../../../src/ngtsc/annotations';
|
||||
import {CycleAnalyzer, ImportGraph} from '../../../src/ngtsc/cycles';
|
||||
import {AbsoluteFsPath, FileSystem, LogicalFileSystem, absoluteFrom, dirname, resolve} from '../../../src/ngtsc/file_system';
|
||||
import {FileSystem, LogicalFileSystem, absoluteFrom, dirname, resolve} from '../../../src/ngtsc/file_system';
|
||||
import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NOOP_DEFAULT_IMPORT_RECORDER, ReferenceEmitter} from '../../../src/ngtsc/imports';
|
||||
import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, LocalMetadataRegistry} from '../../../src/ngtsc/metadata';
|
||||
import {PartialEvaluator} from '../../../src/ngtsc/partial_evaluator';
|
||||
@ -20,6 +20,7 @@ import {CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '
|
||||
import {NgccReflectionHost} from '../host/ngcc_host';
|
||||
import {EntryPointBundle} from '../packages/entry_point_bundle';
|
||||
import {isDefined} from '../utils';
|
||||
import {isWithinPackage} from './util';
|
||||
|
||||
export interface AnalyzedFile {
|
||||
sourceFile: ts.SourceFile;
|
||||
@ -72,6 +73,7 @@ export class DecorationAnalyzer {
|
||||
private host = this.bundle.src.host;
|
||||
private typeChecker = this.bundle.src.program.getTypeChecker();
|
||||
private rootDirs = this.bundle.rootDirs;
|
||||
private packagePath = this.bundle.entryPoint.package;
|
||||
private isCore = this.bundle.isCore;
|
||||
resourceManager = new NgccResourceLoader(this.fs);
|
||||
metaRegistry = new LocalMetadataRegistry();
|
||||
@ -130,6 +132,7 @@ export class DecorationAnalyzer {
|
||||
analyzeProgram(): DecorationAnalyses {
|
||||
const decorationAnalyses = new DecorationAnalyses();
|
||||
const analysedFiles = this.program.getSourceFiles()
|
||||
.filter(sourceFile => isWithinPackage(this.packagePath, sourceFile))
|
||||
.map(sourceFile => this.analyzeFile(sourceFile))
|
||||
.filter(isDefined);
|
||||
analysedFiles.forEach(analysedFile => this.resolveFile(analysedFile));
|
||||
|
@ -6,7 +6,9 @@
|
||||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
import * as ts from 'typescript';
|
||||
import {AbsoluteFsPath} from '../../../src/ngtsc/file_system';
|
||||
import {NgccReflectionHost, SwitchableVariableDeclaration} from '../host/ngcc_host';
|
||||
import {isWithinPackage} from './util';
|
||||
|
||||
export interface SwitchMarkerAnalysis {
|
||||
sourceFile: ts.SourceFile;
|
||||
@ -21,7 +23,7 @@ export const SwitchMarkerAnalyses = Map;
|
||||
* that will be replaced.
|
||||
*/
|
||||
export class SwitchMarkerAnalyzer {
|
||||
constructor(private host: NgccReflectionHost) {}
|
||||
constructor(private host: NgccReflectionHost, private packagePath: AbsoluteFsPath) {}
|
||||
/**
|
||||
* Analyze the files in the program to identify declarations that contain R3
|
||||
* switch markers.
|
||||
@ -32,12 +34,14 @@ export class SwitchMarkerAnalyzer {
|
||||
*/
|
||||
analyzeProgram(program: ts.Program): SwitchMarkerAnalyses {
|
||||
const analyzedFiles = new SwitchMarkerAnalyses();
|
||||
program.getSourceFiles().forEach(sourceFile => {
|
||||
const declarations = this.host.getSwitchableDeclarations(sourceFile);
|
||||
if (declarations.length) {
|
||||
analyzedFiles.set(sourceFile, {sourceFile, declarations});
|
||||
}
|
||||
});
|
||||
program.getSourceFiles()
|
||||
.filter(sourceFile => isWithinPackage(this.packagePath, sourceFile))
|
||||
.forEach(sourceFile => {
|
||||
const declarations = this.host.getSwitchableDeclarations(sourceFile);
|
||||
if (declarations.length) {
|
||||
analyzedFiles.set(sourceFile, {sourceFile, declarations});
|
||||
}
|
||||
});
|
||||
return analyzedFiles;
|
||||
}
|
||||
}
|
||||
|
13
packages/compiler-cli/ngcc/src/analysis/util.ts
Normal file
13
packages/compiler-cli/ngcc/src/analysis/util.ts
Normal file
@ -0,0 +1,13 @@
|
||||
/**
|
||||
* @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 * as ts from 'typescript';
|
||||
import {AbsoluteFsPath, absoluteFromSourceFile, relative} from '../../../src/ngtsc/file_system';
|
||||
|
||||
export function isWithinPackage(packagePath: AbsoluteFsPath, sourceFile: ts.SourceFile): boolean {
|
||||
return !relative(packagePath, absoluteFromSourceFile(sourceFile)).startsWith('..');
|
||||
}
|
Reference in New Issue
Block a user