Skip to content

Commit 04acc6b

Browse files
crisbetothePunderWoman
authored andcommitted
fix(compiler-cli): don't emit empty providers array (#46301)
Saves us some bytes by not emitting `providers` in `defineInjector`. While the amount of bytes isn't huge, I think that this change is worthwhile, because `ng generate` currently generates `providers: []` with every `NgModule` which users can forget to remove. PR Close #46301
1 parent 0410c09 commit 04acc6b

File tree

8 files changed

+126
-10
lines changed

8 files changed

+126
-10
lines changed

packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -332,11 +332,16 @@ export class NgModuleDecoratorHandler implements
332332
};
333333

334334
const rawProviders = ngModule.has('providers') ? ngModule.get('providers')! : null;
335-
const wrapperProviders = rawProviders !== null ?
336-
new WrappedNodeExpr(
337-
this.annotateForClosureCompiler ? wrapFunctionExpressionsInParens(rawProviders) :
338-
rawProviders) :
339-
null;
335+
let wrappedProviders: WrappedNodeExpr<ts.Expression>|null = null;
336+
337+
// In most cases the providers will be an array literal. Check if it has any elements
338+
// and don't include the providers if it doesn't which saves us a few bytes.
339+
if (rawProviders !== null &&
340+
(!ts.isArrayLiteralExpression(rawProviders) || rawProviders.elements.length > 0)) {
341+
wrappedProviders = new WrappedNodeExpr(
342+
this.annotateForClosureCompiler ? wrapFunctionExpressionsInParens(rawProviders) :
343+
rawProviders);
344+
}
340345

341346
const topLevelImports: TopLevelImportedExpression[] = [];
342347
if (ngModule.has('imports')) {
@@ -378,7 +383,7 @@ export class NgModuleDecoratorHandler implements
378383
name,
379384
type,
380385
internalType,
381-
providers: wrapperProviders,
386+
providers: wrappedProviders,
382387
};
383388

384389
const factoryMetadata: R3FactoryMetadata = {

packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/ng_modules/GOLDEN_PARTIAL.js

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,3 +584,58 @@ export declare class ForwardModule {
584584
static ɵinj: i0.ɵɵInjectorDeclaration<ForwardModule>;
585585
}
586586

587+
/****************************************************************************************************
588+
* PARTIAL FILE: empty_fields.js
589+
****************************************************************************************************/
590+
import { NgModule } from '@angular/core';
591+
import * as i0 from "@angular/core";
592+
export class FooModule {
593+
}
594+
FooModule.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: FooModule, deps: [], target: i0.ɵɵFactoryTarget.NgModule });
595+
FooModule.ɵmod = i0.ɵɵngDeclareNgModule({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: FooModule });
596+
FooModule.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: FooModule });
597+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: FooModule, decorators: [{
598+
type: NgModule,
599+
args: [{
600+
providers: [],
601+
declarations: [],
602+
imports: [],
603+
}]
604+
}] });
605+
606+
/****************************************************************************************************
607+
* PARTIAL FILE: empty_fields.d.ts
608+
****************************************************************************************************/
609+
import * as i0 from "@angular/core";
610+
export declare class FooModule {
611+
static ɵfac: i0.ɵɵFactoryDeclaration<FooModule, never>;
612+
static ɵmod: i0.ɵɵNgModuleDeclaration<FooModule, never, never, never>;
613+
static ɵinj: i0.ɵɵInjectorDeclaration<FooModule>;
614+
}
615+
616+
/****************************************************************************************************
617+
* PARTIAL FILE: variable_providers.js
618+
****************************************************************************************************/
619+
import { InjectionToken, NgModule } from '@angular/core';
620+
import * as i0 from "@angular/core";
621+
const PROVIDERS = [{ provide: new InjectionToken('token'), useValue: 1 }];
622+
export class FooModule {
623+
}
624+
FooModule.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: FooModule, deps: [], target: i0.ɵɵFactoryTarget.NgModule });
625+
FooModule.ɵmod = i0.ɵɵngDeclareNgModule({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: FooModule });
626+
FooModule.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: FooModule, providers: PROVIDERS });
627+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: FooModule, decorators: [{
628+
type: NgModule,
629+
args: [{ providers: PROVIDERS }]
630+
}] });
631+
632+
/****************************************************************************************************
633+
* PARTIAL FILE: variable_providers.d.ts
634+
****************************************************************************************************/
635+
import * as i0 from "@angular/core";
636+
export declare class FooModule {
637+
static ɵfac: i0.ɵɵFactoryDeclaration<FooModule, never>;
638+
static ɵmod: i0.ɵɵNgModuleDeclaration<FooModule, never, never, never>;
639+
static ɵinj: i0.ɵɵInjectorDeclaration<FooModule>;
640+
}
641+

packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/ng_modules/TEST_CASES.json

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,34 @@
164164
"angularCompilerOptions": {
165165
"linkerJitMode": true
166166
}
167+
},
168+
{
169+
"description": "should not pass along empty array fields to the declaration",
170+
"inputFiles": [
171+
"empty_fields.ts"
172+
],
173+
"expectations": [
174+
{
175+
"failureMessage": "Invalid injector definition",
176+
"files": [
177+
"empty_fields.js"
178+
]
179+
}
180+
]
181+
},
182+
{
183+
"description": "should handle providers passed in as a variable",
184+
"inputFiles": [
185+
"variable_providers.ts"
186+
],
187+
"expectations": [
188+
{
189+
"failureMessage": "Invalid injector definition",
190+
"files": [
191+
"variable_providers.js"
192+
]
193+
}
194+
]
167195
}
168196
]
169-
}
197+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
export class FooModule {}
2+
FooModule.ɵfac = ;
3+
FooModule.ɵmod = ;
4+
FooModule.ɵinj = /*@__PURE__*/ i0.ɵɵdefineInjector({});
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import {NgModule} from '@angular/core';
2+
3+
@NgModule({
4+
providers: [],
5+
declarations: [],
6+
imports: [],
7+
})
8+
export class FooModule {
9+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
const PROVIDERS = [{provide: new InjectionToken('token'), useValue: 1}];
2+
export class FooModule {}
3+
FooModule.ɵfac = ;
4+
FooModule.ɵmod = ;
5+
FooModule.ɵinj = /*@__PURE__*/ i0.ɵɵdefineInjector({providers: PROVIDERS});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import {InjectionToken, NgModule} from '@angular/core';
2+
3+
const PROVIDERS = [{provide: new InjectionToken('token'), useValue: 1}];
4+
5+
@NgModule({providers: PROVIDERS})
6+
export class FooModule {
7+
}

packages/compiler/src/jit_compiler_facade.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,9 @@ export class CompilerFacadeImpl implements CompilerFacade {
107107
name: facade.name,
108108
type: wrapReference(facade.type),
109109
internalType: new WrappedNodeExpr(facade.type),
110-
providers: new WrappedNodeExpr(facade.providers),
110+
providers: facade.providers && facade.providers.length > 0 ?
111+
new WrappedNodeExpr(facade.providers) :
112+
null,
111113
imports: facade.imports.map(i => new WrappedNodeExpr(i)),
112114
};
113115
const res = compileInjector(meta);
@@ -661,8 +663,9 @@ function convertDeclareInjectorFacadeToMetadata(declaration: R3DeclareInjectorFa
661663
name: declaration.type.name,
662664
type: wrapReference(declaration.type),
663665
internalType: new WrappedNodeExpr(declaration.type),
664-
providers: declaration.providers !== undefined ? new WrappedNodeExpr(declaration.providers) :
665-
null,
666+
providers: declaration.providers !== undefined && declaration.providers.length > 0 ?
667+
new WrappedNodeExpr(declaration.providers) :
668+
null,
666669
imports: declaration.imports !== undefined ?
667670
declaration.imports.map(i => new WrappedNodeExpr(i)) :
668671
[],

0 commit comments

Comments
 (0)