Skip to content

Commit 8ecfd71

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 c2ba7c4 commit 8ecfd71

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
@@ -316,11 +316,16 @@ export class NgModuleDecoratorHandler implements
316316
};
317317

318318
const rawProviders = ngModule.has('providers') ? ngModule.get('providers')! : null;
319-
const wrapperProviders = rawProviders !== null ?
320-
new WrappedNodeExpr(
321-
this.annotateForClosureCompiler ? wrapFunctionExpressionsInParens(rawProviders) :
322-
rawProviders) :
323-
null;
319+
let wrappedProviders: WrappedNodeExpr<ts.Expression>|null = null;
320+
321+
// In most cases the providers will be an array literal. Check if it has any elements
322+
// and don't include the providers if it doesn't which saves us a few bytes.
323+
if (rawProviders !== null &&
324+
(!ts.isArrayLiteralExpression(rawProviders) || rawProviders.elements.length > 0)) {
325+
wrappedProviders = new WrappedNodeExpr(
326+
this.annotateForClosureCompiler ? wrapFunctionExpressionsInParens(rawProviders) :
327+
rawProviders);
328+
}
324329

325330
const topLevelImports: TopLevelImportedExpression[] = [];
326331
if (ngModule.has('imports')) {
@@ -362,7 +367,7 @@ export class NgModuleDecoratorHandler implements
362367
name,
363368
type,
364369
internalType,
365-
providers: wrapperProviders,
370+
providers: wrappedProviders,
366371
};
367372

368373
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+
}
Lines changed: 5 additions & 0 deletions
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});
Lines changed: 7 additions & 0 deletions
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);
@@ -659,8 +661,9 @@ function convertDeclareInjectorFacadeToMetadata(declaration: R3DeclareInjectorFa
659661
name: declaration.type.name,
660662
type: wrapReference(declaration.type),
661663
internalType: new WrappedNodeExpr(declaration.type),
662-
providers: declaration.providers !== undefined ? new WrappedNodeExpr(declaration.providers) :
663-
null,
664+
providers: declaration.providers !== undefined && declaration.providers.length > 0 ?
665+
new WrappedNodeExpr(declaration.providers) :
666+
null,
664667
imports: declaration.imports !== undefined ?
665668
declaration.imports.map(i => new WrappedNodeExpr(i)) :
666669
[],

0 commit comments

Comments
 (0)