Skip to content

Commit 235a235

Browse files
alxhubmhevery
authored andcommitted
feat: change @Injectable() to support tree-shakeable tokens (angular#22005)
This commit bundles 3 important changes, with the goal of enabling tree-shaking of services which are never injected. Ordinarily, this tree-shaking is prevented by the existence of a hard dependency on the service by the module in which it is declared. Firstly, @Injectable() is modified to accept a 'scope' parameter, which points to an @NgModule(). This reverses the dependency edge, permitting the module to not depend on the service which it "provides". Secondly, the runtime is modified to understand the new relationship created above. When a module receives a request to inject a token, and cannot find that token in its list of providers, it will then look at the token for a special ngInjectableDef field which indicates which module the token is scoped to. If that module happens to be in the injector, it will behave as if the token itself was in the injector to begin with. Thirdly, the compiler is modified to read the @Injectable() metadata and to generate the special ngInjectableDef field as part of TS compilation, using the PartialModules system. Additionally, this commit adds several unit and integration tests of various flavors to test this change. PR Close angular#22005
1 parent 2d5e7d1 commit 235a235

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+1752
-227
lines changed

integration/_payload-limits.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@
33
"master": {
44
"uncompressed": {
55
"inline": 1447,
6-
"main": 154185,
6+
"main": 159944,
77
"polyfills": 59179
88
}
99
}
1010
},
1111
"hello_world__closure": {
1212
"master": {
1313
"uncompressed": {
14-
"bundle": 101744
14+
"bundle": 105779
1515
}
1616
}
1717
},
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
{
2+
"name": "angular-integration",
3+
"version": "0.0.0",
4+
"license": "MIT",
5+
"dependencies": {
6+
"@angular/animations": "file:../../dist/packages-dist/animations",
7+
"@angular/common": "file:../../dist/packages-dist/common",
8+
"@angular/compiler": "file:../../dist/packages-dist/compiler",
9+
"@angular/compiler-cli": "file:../../dist/packages-dist/compiler-cli",
10+
"@angular/core": "file:../../dist/packages-dist/core",
11+
"@angular/http": "file:../../dist/packages-dist/http",
12+
"@angular/platform-browser": "file:../../dist/packages-dist/platform-browser",
13+
"@angular/platform-browser-dynamic": "file:../../dist/packages-dist/platform-browser-dynamic",
14+
"@angular/platform-server": "file:../../dist/packages-dist/platform-server",
15+
"@types/node": "^9.4.0",
16+
"rxjs": "file:../../node_modules/rxjs",
17+
"typescript": "file:../../node_modules/typescript",
18+
"zone.js": "file:../../node_modules/zone.js"
19+
},
20+
"devDependencies": {
21+
"@types/jasmine": "2.5.41",
22+
"concurrently": "3.4.0",
23+
"lite-server": "2.2.2",
24+
"protractor": "file:../../node_modules/protractor"
25+
},
26+
"scripts": {
27+
"test": "./test.sh"
28+
}
29+
}

integration/injectable-def/src/app.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import {Component, NgModule} from '@angular/core';
2+
import {BrowserModule} from '@angular/platform-browser';
3+
import {ServerModule} from '@angular/platform-server';
4+
import {Lib2Module} from 'lib2_built';
5+
6+
@Component({
7+
selector: 'test-app',
8+
template: '<test-cmp></test-cmp>',
9+
})
10+
export class TestApp {}
11+
12+
@NgModule({
13+
declarations: [TestApp],
14+
bootstrap: [TestApp],
15+
imports: [
16+
Lib2Module,
17+
BrowserModule.withServerTransition({appId: 'appId'}),
18+
ServerModule,
19+
],
20+
})
21+
export class AppModule {}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import {Injectable, NgModule} from '@angular/core';
2+
3+
@NgModule({})
4+
export class Lib1Module {}
5+
6+
@Injectable({scope: Lib1Module})
7+
export class Service {
8+
static instance = 0;
9+
readonly instance = Service.instance++;
10+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import {Component, Injector, NgModule} from '@angular/core';
2+
import {Lib1Module, Service} from 'lib1_built';
3+
4+
@Component({
5+
selector: 'test-cmp',
6+
template: '{{instance1}}:{{instance2}}',
7+
})
8+
export class TestCmp {
9+
instance1: number;
10+
instance2: number;
11+
12+
constructor(service: Service, injector: Injector) {
13+
this.instance1 = service.instance;
14+
this.instance2 = injector.get(Service).instance;
15+
}
16+
}
17+
18+
@NgModule({
19+
declarations: [TestCmp],
20+
exports: [TestCmp],
21+
imports: [Lib1Module],
22+
})
23+
export class Lib2Module {}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import 'zone.js/dist/zone-node';
2+
3+
import {enableProdMode} from '@angular/core';
4+
import {renderModuleFactory} from '@angular/platform-server';
5+
import {AppModuleNgFactory} from './app.ngfactory';
6+
7+
enableProdMode();
8+
renderModuleFactory(AppModuleNgFactory, {
9+
document: '<test-app></test-app>',
10+
url: '/',
11+
}).then(html => {
12+
if (/>0:0</.test(html)) {
13+
process.exit(0);
14+
} else {
15+
console.error('html was', html);
16+
process.exit(1);
17+
}
18+
}).catch(err => {
19+
console.error(err);
20+
process.exit(2);
21+
})
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "lib1_built",
3+
"version": "0.0.0",
4+
"license": "MIT",
5+
"main": "./src/index.js",
6+
"typings": "./src/index.d.ts"
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "lib2_built",
3+
"version": "0.0.0",
4+
"license": "MIT",
5+
"main": "./src/index.js",
6+
"typings": "./src/index.d.ts"
7+
}

integration/injectable-def/test.sh

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#!/bin/bash
2+
set -e -x
3+
4+
NPM_BIN=$(npm bin)
5+
PATH="$PATH:${NPM_BIN}"
6+
7+
rm -rf node_modules/lib1_built node_modules/lib2_built dist/
8+
9+
ngc -p tsconfig-lib1.json
10+
cp src/package-lib1.json node_modules/lib1_built/package.json
11+
12+
ngc -p tsconfig-lib2.json
13+
cp src/package-lib2.json node_modules/lib2_built/package.json
14+
15+
ngc -p tsconfig-app.json
16+
17+
node ./dist/src/main.js
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"compilerOptions": {
3+
"target": "es5",
4+
"module": "commonjs",
5+
"moduleResolution": "node",
6+
"lib": ["es2015", "dom"],
7+
"experimentalDecorators": true,
8+
"emitDecoratorMetadata": true,
9+
"outDir": "dist",
10+
"types": ["node"],
11+
"rootDir": "."
12+
},
13+
"files": [
14+
"src/app.ts",
15+
"src/main.ts"
16+
]
17+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
{
2+
"compilerOptions": {
3+
"target": "es5",
4+
"module": "commonjs",
5+
"declaration": true,
6+
"moduleResolution": "node",
7+
"lib": ["es2015", "dom"],
8+
"experimentalDecorators": true,
9+
"emitDecoratorMetadata": true,
10+
"outDir": "node_modules/lib1_built",
11+
"types": [],
12+
"rootDir": "."
13+
},
14+
"files": [
15+
"src/lib1.ts"
16+
],
17+
"angularCompilerOptions": {
18+
"skipTemplateCodegen": true,
19+
"flatModuleOutFile": "index.js",
20+
"flatModuleId": "lib1_built"
21+
}
22+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
{
2+
"compilerOptions": {
3+
"target": "es5",
4+
"module": "commonjs",
5+
"declaration": true,
6+
"moduleResolution": "node",
7+
"lib": ["es2015", "dom"],
8+
"experimentalDecorators": true,
9+
"emitDecoratorMetadata": true,
10+
"outDir": "node_modules/lib2_built",
11+
"types": [],
12+
"rootDir": "."
13+
},
14+
"files": [
15+
"src/lib2.ts"
16+
],
17+
"angularCompilerOptions": {
18+
"skipTemplateCodegen": true,
19+
"flatModuleId": "lib2_built",
20+
"flatModuleOutFile": "index.js"
21+
}
22+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package(default_visibility = ["//visibility:public"])
2+
3+
load("//tools:defaults.bzl", "ng_module", "ts_library")
4+
load("@build_bazel_rules_nodejs//:defs.bzl", "jasmine_node_test")
5+
6+
ng_module(
7+
name = "app",
8+
srcs = glob(
9+
[
10+
"src/**/*.ts",
11+
],
12+
),
13+
module_name = "app_built",
14+
deps = [
15+
"//packages/compiler-cli/integrationtest/bazel/injectable_def/lib2",
16+
"//packages/core",
17+
"//packages/platform-browser",
18+
"//packages/platform-server",
19+
"@rxjs",
20+
],
21+
)
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {Component, NgModule} from '@angular/core';
10+
import {BrowserModule} from '@angular/platform-browser';
11+
import {ServerModule} from '@angular/platform-server';
12+
import {Lib2Module} from 'lib2_built/module';
13+
14+
@Component({
15+
selector: 'id-app',
16+
template: '<lib2-cmp></lib2-cmp>',
17+
})
18+
export class AppComponent {
19+
}
20+
21+
@NgModule({
22+
imports: [
23+
Lib2Module,
24+
BrowserModule.withServerTransition({appId: 'id-app'}),
25+
ServerModule,
26+
],
27+
declarations: [AppComponent],
28+
bootstrap: [AppComponent],
29+
})
30+
export class BasicAppModule {
31+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {Component, Injectable, NgModule, Optional, Self} from '@angular/core';
10+
import {BrowserModule} from '@angular/platform-browser';
11+
import {ServerModule} from '@angular/platform-server';
12+
13+
@Injectable()
14+
export class Service {
15+
}
16+
17+
@Component({
18+
selector: 'hierarchy-app',
19+
template: '<child-cmp></child-cmp>',
20+
providers: [Service],
21+
})
22+
export class AppComponent {
23+
}
24+
25+
@Component({
26+
selector: 'child-cmp',
27+
template: '{{found}}',
28+
})
29+
export class ChildComponent {
30+
found: boolean;
31+
32+
constructor(@Optional() @Self() service: Service|null) { this.found = !!service; }
33+
}
34+
35+
@NgModule({
36+
imports: [
37+
BrowserModule.withServerTransition({appId: 'hierarchy-app'}),
38+
ServerModule,
39+
],
40+
declarations: [AppComponent, ChildComponent],
41+
bootstrap: [AppComponent],
42+
})
43+
export class HierarchyAppModule {
44+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {Component, Injectable, NgModule, Optional, Self} from '@angular/core';
10+
import {BrowserModule} from '@angular/platform-browser';
11+
import {ServerModule} from '@angular/platform-server';
12+
13+
@Injectable()
14+
export class NormalService {
15+
constructor(@Optional() @Self() readonly shakeable: ShakeableService|null) {}
16+
}
17+
18+
@Component({
19+
selector: 'self-app',
20+
template: '{{found}}',
21+
})
22+
export class AppComponent {
23+
found: boolean;
24+
constructor(service: NormalService) { this.found = !!service.shakeable; }
25+
}
26+
27+
@NgModule({
28+
imports: [
29+
BrowserModule.withServerTransition({appId: 'id-app'}),
30+
ServerModule,
31+
],
32+
declarations: [AppComponent],
33+
bootstrap: [AppComponent],
34+
providers: [NormalService],
35+
})
36+
export class SelfAppModule {
37+
}
38+
39+
@Injectable({scope: SelfAppModule})
40+
export class ShakeableService {
41+
}

0 commit comments

Comments
 (0)