Skip to content

Commit a290168

Browse files
devversiondevversion
authored
refactor(list): use light-weight tokens for injecting parent lists (#19568)
* build: add infrastructure for performing size golden tests * refactor(list): use light-weight tokens for injecting parent lists Angular Material list items currently optionally inject a parent `MatNavList` or `MatList`. This has the downside of retaining these tokens at runtime because they are used for dependency injection as values. We can improve this by using so-called light-weight tokens. These allow us to continue injecting parent list or nav-lists, but without the risk of retaining the `MatList` and `MatNavList` classes with their factories. This was already the case before Angular v9 with View Engine, but the issue significance increases with Ivy where factories are now directly attached to the classes (such as `MatList` or `MatNavList`). Using light-weight tokens avoids this issue and gives us an additional size improvement. Notably this won't be an improvement when an application uses both the nav-list and standard `MatList`. Related to https://github.com/angular/angular-cli/issues/16866. More context on light-weight tokens in: https://hackmd.io/@mhevery/SyqDjUlrU#.
1 parent d15f19e commit a290168

File tree

18 files changed

+325
-30
lines changed

18 files changed

+325
-30
lines changed

‎.circleci/config.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -535,8 +535,8 @@ jobs:
535535
- *setup_bazel_remote_execution
536536
- *yarn_install
537537
- *setup_bazel_binary
538-
# Integration tests run with --config=view-engine because we release with View Engine.
539-
- run: bazel test integration/... --build_tests_only --config=view-engine
538+
- run: yarn integration-tests
539+
- run: yarn integration-tests:view-engine
540540

541541
# ----------------------------------------------------------------------------
542542
# Job that runs all Bazel tests against material-components-web@canary

‎.github/CODEOWNERS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,3 +320,5 @@
320320
/.vscode/** @angular/dev-infra-components @mmalerba
321321
/src/* @jelbourn
322322
/goldens/ts-circular-deps.json @angular/dev-infra-components
323+
/goldens/size-test.json @jelbourn @mmalerba @crisbeto
324+
/goldens/* @angular/dev-infra-components

‎WORKSPACE

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
88
# Add NodeJS rules
99
http_archive(
1010
name = "build_bazel_rules_nodejs",
11-
sha256 = "d14076339deb08e5460c221fae5c5e9605d2ef4848eee1f0c81c9ffdc1ab31c1",
12-
urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/1.6.1/rules_nodejs-1.6.1.tar.gz"],
11+
sha256 = "84abf7ac4234a70924628baa9a73a5a5cbad944c4358cf9abdb4aab29c9a5b77",
12+
urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/1.7.0/rules_nodejs-1.7.0.tar.gz"],
1313
)
1414

1515
# Add sass rules

‎goldens/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
exports_files([
2+
"size-test.json",
3+
])

‎goldens/size-test.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"material/list": 215881
3+
}

‎integration/size-test/BUILD.bazel

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
load("//tools:defaults.bzl", "ts_library")
2+
3+
package(default_visibility = ["//visibility:public"])
4+
5+
exports_files([
6+
"rollup.config.js",
7+
"terser-config.json",
8+
])
9+
10+
ts_library(
11+
name = "check-size",
12+
srcs = ["check-size.ts"],
13+
deps = [
14+
"@npm//@types/node",
15+
"@npm//chalk",
16+
],
17+
)

‎integration/size-test/check-size.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/*
2+
* Script that measures the size of a given test file and compares it
3+
* with an entry in a file size golden. If the size deviates by certain
4+
* amount, the script will fail with a non-zero exit code.
5+
*/
6+
7+
import chalk from 'chalk';
8+
import {readFileSync, statSync, writeFileSync} from 'fs';
9+
10+
/**
11+
* Absolute byte deviation from the expected value that is allowed. If the
12+
* size deviates by 500 bytes of the expected value, the script will fail.
13+
*/
14+
const ABSOLUTE_BYTE_THRESHOLD = 500;
15+
/**
16+
* Percentage deviation from the expected value that is allowed. If the
17+
* size deviates by 1% of the expected value, the script will fail.
18+
*/
19+
const PERCENTAGE_DEVIATION_THRESHOLD = 1;
20+
21+
/**
22+
* Extracted command line arguments specified by the Bazel NodeJS binaries:
23+
* - `testId`: Unique id for the given test file that is used as key in the golden.
24+
* - `testFileRootpath`: Bazel rootpath that resolves to the test file that should be measured.
25+
* - `isApprove`: Whether the script runs in approve mode, and the golden should be updated
26+
* with the actual measured size.
27+
*/
28+
const [testId, testFileRootpath, isApprove] = process.argv.slice(2);
29+
const testFilePath = require.resolve(`angular_material/${testFileRootpath}`);
30+
const goldenFilePath = require.resolve('../../goldens/size-test.json');
31+
32+
const golden = JSON.parse(readFileSync(goldenFilePath, 'utf8'));
33+
const fileStat = statSync(testFilePath);
34+
const actualSize = fileStat.size;
35+
36+
// If in approve mode, update the golden to reflect the new size.
37+
if (isApprove) {
38+
golden[testId] = actualSize;
39+
writeFileSync(goldenFilePath, JSON.stringify(golden, null, 2));
40+
console.info(chalk.green(`Approved golden size for "${testId}"`));
41+
process.exit(0);
42+
}
43+
44+
// If no size has been collected for the test id, report an error.
45+
if (golden[testId] === undefined) {
46+
console.error(`No golden size for "${testId}". Please create a new entry.`);
47+
printApproveCommand();
48+
process.exit(1);
49+
}
50+
51+
const expectedSize = Number(golden[testId]);
52+
const absoluteSizeDiff = Math.abs(actualSize - expectedSize);
53+
const deviatedByPercentage =
54+
absoluteSizeDiff > (expectedSize * PERCENTAGE_DEVIATION_THRESHOLD / 100);
55+
const deviatedByAbsoluteDiff = absoluteSizeDiff > ABSOLUTE_BYTE_THRESHOLD;
56+
57+
// Always print the expected and actual size so that it's easier to find culprit
58+
// commits when the size slowly moves toward the threshold boundaries.
59+
console.info(chalk.yellow(`Expected: ${expectedSize}, but got: ${actualSize}`));
60+
61+
if (deviatedByPercentage) {
62+
console.error(`Actual size deviates by more than 1% of the expected size. `);
63+
printApproveCommand();
64+
process.exit(1);
65+
} else if (deviatedByAbsoluteDiff) {
66+
console.error(`Actual size deviates by more than 500 bytes from the expected.`);
67+
printApproveCommand();
68+
process.exit(1);
69+
}
70+
71+
/** Prints the command for approving the current test. */
72+
function printApproveCommand() {
73+
console.info(chalk.yellow('You can approve the golden by running the following command:'));
74+
console.info(chalk.yellow(` bazel run ${process.env.BAZEL_TARGET}.approve`));
75+
}

‎integration/size-test/index.bzl

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "nodejs_test")
2+
load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
3+
load("@npm_bazel_terser//:index.bzl", "terser_minified")
4+
load("//tools:defaults.bzl", "ng_module")
5+
6+
"""
7+
Performs size measurements for the specified file. The file will be built as part
8+
of a `ng_module` and then will be optimized with build-optimizer, rollup and Terser.
9+
10+
The resulting size will be validated against a golden file to ensure that we don't
11+
regress in payload size, or that we can improvements to payload size.
12+
"""
13+
14+
def size_test(name, file, deps):
15+
# Generates an unique id for the given size test. e.g. if the macro is called
16+
# within the `integration/size-test/material` package with `name = list`, then
17+
# the unique id will be set to `material/list`.
18+
test_id = "%s/%s" % (native.package_name()[len("integration/size-test/"):], name)
19+
20+
ng_module(
21+
name = "%s_lib" % name,
22+
srcs = ["%s.ts" % name],
23+
testonly = True,
24+
deps = [
25+
"@npm//@angular/core",
26+
"@npm//@angular/platform-browser-dynamic",
27+
] + deps,
28+
)
29+
30+
rollup_bundle(
31+
name = "%s_bundle" % name,
32+
config_file = "//integration/size-test:rollup.config.js",
33+
testonly = True,
34+
entry_points = {
35+
(file): "%s_bundled" % name,
36+
},
37+
deps = [
38+
":%s_lib" % name,
39+
"@npm//rollup-plugin-node-resolve",
40+
"@npm//@angular-devkit/build-optimizer",
41+
],
42+
sourcemap = "false",
43+
)
44+
45+
terser_minified(
46+
testonly = True,
47+
name = "%s_bundle_min" % name,
48+
src = ":%s_bundle" % name,
49+
config_file = "//integration/size-test:terser-config.json",
50+
sourcemap = False,
51+
)
52+
53+
nodejs_test(
54+
name = name,
55+
data = [
56+
"//goldens:size-test.json",
57+
"//integration/size-test:check-size",
58+
":%s_bundle_min" % name,
59+
],
60+
entry_point = "//integration/size-test:check-size.ts",
61+
args = [test_id, "$(rootpath :%s_bundle_min)" % name],
62+
)
63+
64+
nodejs_binary(
65+
name = "%s.approve" % name,
66+
testonly = True,
67+
data = [
68+
"//goldens:size-test.json",
69+
"//integration/size-test:check-size",
70+
":%s_bundle_min" % name,
71+
],
72+
entry_point = "//integration/size-test:check-size.ts",
73+
args = [test_id, "$(rootpath :%s_bundle_min)" % name, "true"],
74+
)

‎integration/size-test/material/BUILD.bazel

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
load("//integration/size-test:index.bzl", "size_test")
2+
3+
size_test(
4+
name = "list",
5+
file = "list.ts",
6+
deps = ["//src/material/list"],
7+
)

‎integration/size-test/material/list.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import {Component, NgModule} from '@angular/core';
2+
import {platformBrowser} from '@angular/platform-browser';
3+
import {MatListModule} from '@angular/material/list';
4+
5+
/**
6+
* Basic component using `MatNavList` and `MatListItem`. Other parts of the list
7+
* module such as `MatList`, `MatSelectionList` or `MatListOption` are not used
8+
* and should be tree-shaken away.
9+
*/
10+
@Component({
11+
template: `
12+
<mat-nav-list>
13+
<mat-list-item>
14+
hello
15+
</mat-list-item>
16+
</mat-nav-list>
17+
`,
18+
})
19+
export class TestComponent {}
20+
21+
@NgModule({
22+
imports: [MatListModule],
23+
declarations: [TestComponent],
24+
bootstrap: [TestComponent],
25+
})
26+
export class AppModule {}
27+
28+
platformBrowser().bootstrapModule(AppModule);

‎integration/size-test/rollup.config.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
const {buildOptimizer} = require('@angular-devkit/build-optimizer');
2+
const node = require('rollup-plugin-node-resolve');
3+
4+
const buildOptimizerPlugin = {
5+
name: 'build-optimizer',
6+
transform: (content, id) => {
7+
const {content: code, sourceMap: map} = buildOptimizer({
8+
content,
9+
inputFilePath: id,
10+
emitSourceMap: true,
11+
isSideEffectFree: true,
12+
isAngularCoreFile: false,
13+
});
14+
if (!code) {
15+
return null;
16+
}
17+
if (!map) {
18+
throw new Error('No sourcemap produced by build optimizer');
19+
}
20+
return {code, map};
21+
},
22+
};
23+
24+
module.exports = {
25+
plugins: [
26+
buildOptimizerPlugin,
27+
node({
28+
mainFields: ['es2015_ivy_ngcc', 'module_ivy_ngcc','es2015', 'module'],
29+
}),
30+
],
31+
};

‎integration/size-test/terser-config.json

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"output": {
3+
"ecma": "es2015",
4+
"comments": false
5+
},
6+
"compress": {
7+
"global_defs": {
8+
"ngDevMode": false,
9+
"ngI18nClosureMode": false,
10+
"ngJitMode": false
11+
},
12+
"passes": 3,
13+
"pure_getters": true
14+
},
15+
"mangle": true
16+
}

‎integration/size-test/tsconfig.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"compilerOptions": {
3+
"experimentalDecorators": true,
4+
"lib": ["es2015"],
5+
"types": ["node"]
6+
}
7+
}

‎integration/ts-compat/BUILD.bazel

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ typescript_version_packages = [
4040
],
4141
entry_point = "test.js",
4242
tags = [
43-
"integration",
43+
# These tests run in view engine only as the release packages are
44+
# built with View Engine and we want to reproduce this here.
45+
"view-engine-only",
4446
],
4547
)
4648
for ts_pkg_name in typescript_version_packages

‎package.json

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@
4444
"ts-circular-deps:check": "yarn -s ts-circular-deps check --config ./src/circular-deps-test.conf.js",
4545
"ts-circular-deps:approve": "yarn -s ts-circular-deps approve --config ./src/circular-deps-test.conf.js",
4646
"merge": "ng-dev pr merge",
47-
"approve-api": "node ./scripts/approve-api-golden.js"
47+
"approve-api": "node ./scripts/approve-api-golden.js",
48+
"integration-tests": "bazel test //integration/... --test_tag_filters=-view-engine-only --build_tests_only",
49+
"integration-tests:view-engine": "bazel test //integration/... --test_tag_filters=view-engine-only --build_tests_only --config=view-engine"
4850
},
4951
"version": "10.0.0-rc.1",
5052
"dependencies": {
@@ -79,11 +81,12 @@
7981
"@bazel/bazelisk": "^1.4.0",
8082
"@bazel/buildifier": "^2.2.1",
8183
"@bazel/ibazel": "^0.13.0",
82-
"@bazel/jasmine": "^1.6.0",
83-
"@bazel/karma": "^1.6.0",
84-
"@bazel/protractor": "^1.6.0",
85-
"@bazel/terser": "^1.4.1",
86-
"@bazel/typescript": "^1.6.0",
84+
"@bazel/jasmine": "^1.7.0",
85+
"@bazel/karma": "^1.7.0",
86+
"@bazel/protractor": "^1.7.0",
87+
"@bazel/rollup": "^1.7.0",
88+
"@bazel/terser": "^1.7.0",
89+
"@bazel/typescript": "^1.7.0",
8790
"@firebase/app-types": "^0.3.2",
8891
"@octokit/rest": "16.28.7",
8992
"@schematics/angular": "^10.0.0-rc.2",

‎src/material/list/list.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import {
2222
OnDestroy,
2323
ChangeDetectorRef,
2424
Input,
25+
InjectionToken,
26+
Inject,
2527
} from '@angular/core';
2628
import {
2729
CanDisable,
@@ -48,6 +50,20 @@ class MatListItemBase {}
4850
const _MatListItemMixinBase: CanDisableRippleCtor & typeof MatListItemBase =
4951
mixinDisableRipple(MatListItemBase);
5052

53+
/**
54+
* Injection token that can be used to inject instances of `MatList`. It serves as
55+
* alternative token to the actual `MatList` class which could cause unnecessary
56+
* retention of the class and its component metadata.
57+
*/
58+
export const MAT_LIST = new InjectionToken<MatList>('MatList');
59+
60+
/**
61+
* Injection token that can be used to inject instances of `MatNavList`. It serves as
62+
* alternative token to the actual `MatNavList` class which could cause unnecessary
63+
* retention of the class and its component metadata.
64+
*/
65+
export const MAT_NAV_LIST = new InjectionToken<MatNavList>('MatNavList');
66+
5167
@Component({
5268
selector: 'mat-nav-list',
5369
exportAs: 'matNavList',
@@ -60,6 +76,7 @@ const _MatListItemMixinBase: CanDisableRippleCtor & typeof MatListItemBase =
6076
inputs: ['disableRipple', 'disabled'],
6177
encapsulation: ViewEncapsulation.None,
6278
changeDetection: ChangeDetectionStrategy.OnPush,
79+
providers: [{provide: MAT_NAV_LIST, useExisting: MatNavList}],
6380
})
6481
export class MatNavList extends _MatListMixinBase implements CanDisable, CanDisableRipple,
6582
OnChanges, OnDestroy {
@@ -89,6 +106,7 @@ export class MatNavList extends _MatListMixinBase implements CanDisable, CanDisa
89106
inputs: ['disableRipple', 'disabled'],
90107
encapsulation: ViewEncapsulation.None,
91108
changeDetection: ChangeDetectionStrategy.OnPush,
109+
providers: [{provide: MAT_LIST, useExisting: MatList}],
92110
})
93111
export class MatList extends _MatListMixinBase implements CanDisable, CanDisableRipple, OnChanges,
94112
OnDestroy {
@@ -187,8 +205,8 @@ export class MatListItem extends _MatListItemMixinBase implements AfterContentIn
187205

188206
constructor(private _element: ElementRef<HTMLElement>,
189207
_changeDetectorRef: ChangeDetectorRef,
190-
@Optional() navList?: MatNavList,
191-
@Optional() list?: MatList) {
208+
@Optional() @Inject(MAT_NAV_LIST) navList?: MatNavList,
209+
@Optional() @Inject(MAT_LIST) list?: MatList) {
192210
super();
193211
this._isInteractiveList = !!(navList || (list && list._getListType() === 'action-list'));
194212
this._list = navList || list;

0 commit comments

Comments
 (0)