feat(ivy): exclude declarations from injector imports (#29598)
Prior to this change, a module's imports and exports would be used verbatim as an injectors' imports. This is detrimental for tree-shaking, as a module's exports could reference declarations that would then prevent such declarations from being eligible for tree-shaking. Since an injector actually only needs NgModule references as its imports, we may safely filter out any declarations from the list of module exports. This makes them eligible for tree-shaking once again. PR Close #29598
This commit is contained in:
parent
45c6360e5a
commit
2d372f48db
|
@ -21,7 +21,7 @@
|
|||
"master": {
|
||||
"uncompressed": {
|
||||
"runtime": 1440,
|
||||
"main": 212976,
|
||||
"main": 155609,
|
||||
"polyfills": 43567
|
||||
}
|
||||
}
|
||||
|
|
|
@ -14,7 +14,7 @@ import {DefaultImportRecorder, Reference, ReferenceEmitter} from '../../imports'
|
|||
import {PartialEvaluator, ResolvedValue} from '../../partial_evaluator';
|
||||
import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral, typeNodeToValueExpr} from '../../reflection';
|
||||
import {NgModuleRouteAnalyzer} from '../../routing';
|
||||
import {LocalModuleScopeRegistry} from '../../scope';
|
||||
import {LocalModuleScopeRegistry, ScopeData} from '../../scope';
|
||||
import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../transform';
|
||||
import {getSourceFile} from '../../util/src/typescript';
|
||||
|
||||
|
@ -27,6 +27,7 @@ export interface NgModuleAnalysis {
|
|||
ngInjectorDef: R3InjectorMetadata;
|
||||
metadataStmt: Statement|null;
|
||||
declarations: Reference<ClassDeclaration>[];
|
||||
exports: Reference<ClassDeclaration>[];
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -162,13 +163,13 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
|
|||
new LiteralArrayExpr([]);
|
||||
const rawProviders = ngModule.has('providers') ? ngModule.get('providers') ! : null;
|
||||
|
||||
// At this point, only add the module's imports as the injectors' imports. Any exported modules
|
||||
// are added during `resolve`, as we need scope information to be able to filter out directives
|
||||
// and pipes from the module exports.
|
||||
const injectorImports: WrappedNodeExpr<ts.Expression>[] = [];
|
||||
if (ngModule.has('imports')) {
|
||||
injectorImports.push(new WrappedNodeExpr(ngModule.get('imports') !));
|
||||
}
|
||||
if (ngModule.has('exports')) {
|
||||
injectorImports.push(new WrappedNodeExpr(ngModule.get('exports') !));
|
||||
}
|
||||
|
||||
if (this.routeAnalyzer !== null) {
|
||||
this.routeAnalyzer.add(node.getSourceFile(), name, rawImports, rawExports, rawProviders);
|
||||
|
@ -180,7 +181,7 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
|
|||
deps: getValidConstructorDependencies(
|
||||
node, this.reflector, this.defaultImportRecorder, this.isCore),
|
||||
providers,
|
||||
imports: new LiteralArrayExpr(injectorImports),
|
||||
imports: injectorImports,
|
||||
};
|
||||
|
||||
return {
|
||||
|
@ -188,6 +189,7 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
|
|||
ngModuleDef,
|
||||
ngInjectorDef,
|
||||
declarations: declarationRefs,
|
||||
exports: exportRefs,
|
||||
metadataStmt: generateSetClassMetadataCall(
|
||||
node, this.reflector, this.defaultImportRecorder, this.isCore),
|
||||
},
|
||||
|
@ -198,6 +200,18 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
|
|||
resolve(node: ClassDeclaration, analysis: NgModuleAnalysis): ResolveResult {
|
||||
const scope = this.scopeRegistry.getScopeOfModule(node);
|
||||
const diagnostics = this.scopeRegistry.getDiagnosticsOfModule(node) || undefined;
|
||||
|
||||
// Using the scope information, extend the injector's imports using the modules that are
|
||||
// specified as module exports.
|
||||
if (scope !== null) {
|
||||
const context = getSourceFile(node);
|
||||
for (const exportRef of analysis.exports) {
|
||||
if (isNgModule(exportRef.node, scope.compilation)) {
|
||||
analysis.ngInjectorDef.imports.push(this.refEmitter.emit(exportRef, context));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (scope === null || scope.reexports === null) {
|
||||
return {diagnostics};
|
||||
} else {
|
||||
|
@ -394,6 +408,11 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
|
|||
}
|
||||
}
|
||||
|
||||
function isNgModule(node: ClassDeclaration, compilation: ScopeData): boolean {
|
||||
return !compilation.directives.some(directive => directive.ref.node === node) &&
|
||||
!compilation.pipes.some(pipe => pipe.ref.node === node);
|
||||
}
|
||||
|
||||
function isDeclarationReference(ref: any): ref is Reference<ts.Declaration> {
|
||||
return ref instanceof Reference &&
|
||||
(ts.isClassDeclaration(ref.node) || ts.isFunctionDeclaration(ref.node) ||
|
||||
|
|
|
@ -472,6 +472,66 @@ describe('ngtsc behavioral tests', () => {
|
|||
expect(jsContents).not.toContain('ɵsetNgModuleScope(TestModule,');
|
||||
});
|
||||
|
||||
it('should filter out directives and pipes from module exports in the injector def', () => {
|
||||
env.tsconfig();
|
||||
env.write('test.ts', `
|
||||
import {NgModule} from '@angular/core';
|
||||
import {RouterComp, RouterModule} from '@angular/router';
|
||||
import {Dir, OtherDir, MyPipe, Comp} from './decls';
|
||||
|
||||
@NgModule({
|
||||
declarations: [OtherDir],
|
||||
exports: [OtherDir],
|
||||
})
|
||||
export class OtherModule {}
|
||||
|
||||
const EXPORTS = [Dir, MyPipe, Comp, OtherModule, OtherDir, RouterModule, RouterComp];
|
||||
|
||||
@NgModule({
|
||||
declarations: [Dir, MyPipe, Comp],
|
||||
imports: [OtherModule, RouterModule.forRoot()],
|
||||
exports: [EXPORTS],
|
||||
})
|
||||
export class TestModule {}
|
||||
`);
|
||||
env.write(`decls.ts`, `
|
||||
import {Component, Directive, Pipe} from '@angular/core';
|
||||
|
||||
@Directive({selector: '[dir]'})
|
||||
export class Dir {}
|
||||
|
||||
@Directive({selector: '[other]'})
|
||||
export class OtherDir {}
|
||||
|
||||
@Pipe({name:'pipe'})
|
||||
export class MyPipe {}
|
||||
|
||||
@Component({selector: 'test', template: ''})
|
||||
export class Comp {}
|
||||
`);
|
||||
env.write('node_modules/@angular/router/index.d.ts', `
|
||||
import {ɵComponentDefWithMeta, ModuleWithProviders, ɵNgModuleDefWithMeta} from '@angular/core';
|
||||
|
||||
export declare class RouterComp {
|
||||
static ngComponentDef: ɵComponentDefWithMeta<RouterComp, "lib-cmp", never, {}, {}, never>
|
||||
}
|
||||
|
||||
declare class RouterModule {
|
||||
static forRoot(): ModuleWithProviders<RouterModule>;
|
||||
static ngModuleDef: ɵNgModuleDefWithMeta<RouterModule, [typeof RouterComp], never, [typeof RouterComp]>;
|
||||
}
|
||||
`);
|
||||
|
||||
env.driveMain();
|
||||
|
||||
const jsContents = env.getContents('test.js');
|
||||
expect(jsContents)
|
||||
.toContain(
|
||||
'i0.defineInjector({ factory: function TestModule_Factory(t) ' +
|
||||
'{ return new (t || TestModule)(); }, providers: [], ' +
|
||||
'imports: [[OtherModule, RouterModule.forRoot()],\n OtherModule,\n RouterModule] });');
|
||||
});
|
||||
|
||||
it('should compile NgModules with services without errors', () => {
|
||||
env.tsconfig();
|
||||
env.write('test.ts', `
|
||||
|
|
|
@ -110,8 +110,8 @@ export interface R3InjectorMetadataFacade {
|
|||
name: string;
|
||||
type: any;
|
||||
deps: R3DependencyMetadataFacade[]|null;
|
||||
providers: any;
|
||||
imports: any;
|
||||
providers: any[];
|
||||
imports: any[];
|
||||
}
|
||||
|
||||
export interface R3DirectiveMetadataFacade {
|
||||
|
|
|
@ -73,7 +73,7 @@ export class CompilerFacadeImpl implements CompilerFacade {
|
|||
type: new WrappedNodeExpr(facade.type),
|
||||
deps: convertR3DependencyMetadataArray(facade.deps),
|
||||
providers: new WrappedNodeExpr(facade.providers),
|
||||
imports: new WrappedNodeExpr(facade.imports),
|
||||
imports: facade.imports.map(i => new WrappedNodeExpr(i)),
|
||||
};
|
||||
const res = compileInjector(meta);
|
||||
return this.jitExpression(res.expression, angularCoreEnv, sourceMapUrl, res.statements);
|
||||
|
|
|
@ -191,7 +191,7 @@ export interface R3InjectorMetadata {
|
|||
type: o.Expression;
|
||||
deps: R3DependencyMetadata[]|null;
|
||||
providers: o.Expression;
|
||||
imports: o.Expression;
|
||||
imports: o.Expression[];
|
||||
}
|
||||
|
||||
export function compileInjector(meta: R3InjectorMetadata): R3InjectorDef {
|
||||
|
@ -204,7 +204,7 @@ export function compileInjector(meta: R3InjectorMetadata): R3InjectorDef {
|
|||
const expression = o.importExpr(R3.defineInjector).callFn([mapToMapExpression({
|
||||
factory: result.factory,
|
||||
providers: meta.providers,
|
||||
imports: meta.imports,
|
||||
imports: o.literalArr(meta.imports),
|
||||
})]);
|
||||
const type =
|
||||
new o.ExpressionType(o.importExpr(R3.InjectorDef, [new o.ExpressionType(meta.type)]));
|
||||
|
|
Loading…
Reference in New Issue