Skip to content

Commit bf304e9

Browse files
devversionmmalerba
authored andcommitted
build: validate bazel rollup globals (#17173)
* refactor: remove imports to individual files through module name We can't import individual files through the module name because: * We don't have these listed in the rollup globals * These imports will break in the Angular package (due to limitation in APF). * build: fix rollup globals tslint rule not checking all files Apparently the rollup globals tslint rule did not check all files. Resuling in a few cases where the rollup globals were missing. * build: check bazel rollup globals We currently need to also maintain a list of rollup globals for bazel. Since these globals are impacting the release output we need to check these somehow in order to be sure that we do not accidentally bundle other entry-points into an entry-point.
1 parent 54e64bd commit bf304e9

File tree

14 files changed

+158
-15
lines changed

14 files changed

+158
-15
lines changed

.circleci/config.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,8 +265,16 @@ jobs:
265265
steps:
266266
- *checkout_code
267267
- *restore_cache
268+
- *setup_bazel_ci_config
268269
- *yarn_download
269270
- *yarn_install
271+
- *setup_bazel_binary
272+
273+
- run:
274+
name: Checking rollup globals
275+
command: |
276+
bazel build //:rollup_globals
277+
yarn check-rollup-globals $(bazel info bazel-bin)/rollup_globals.json
270278
271279
- run: ./scripts/circleci/lint-bazel-files.sh
272280
- run: yarn gulp ci:lint

BUILD.bazel

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
11
package(default_visibility = ["//visibility:public"])
22

3+
load("//:rollup-globals.bzl", "ROLLUP_GLOBALS")
4+
35
exports_files(["LICENSE"])
6+
7+
genrule(
8+
name = "rollup_globals",
9+
outs = ["rollup_globals.json"],
10+
cmd = "echo '%s' > $@" % ROLLUP_GLOBALS,
11+
)

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@
3434
"format:ts": "git-clang-format HEAD $(git diff HEAD --name-only | grep -v \"\\.d\\.ts\")",
3535
"format:bazel": "yarn -s bazel:buildifier --lint=fix --mode=fix",
3636
"format": "yarn -s format:ts && yarn -s format:bazel",
37-
"cherry-pick-patch": "ts-node --project tools/cherry-pick-patch/ tools/cherry-pick-patch/cherry-pick-patch.ts"
37+
"cherry-pick-patch": "ts-node --project tools/cherry-pick-patch/ tools/cherry-pick-patch/cherry-pick-patch.ts",
38+
"check-rollup-globals": "ts-node --project scripts/ scripts/check-rollup-globals.ts"
3839
},
3940
"version": "8.2.0",
4041
"requiredAngularVersion": "^8.0.0 || ^9.0.0-0",

rollup-globals.bzl

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,33 @@ load(
77
"MATERIAL_EXPERIMENTAL_TESTING_ENTRYPOINTS",
88
)
99

10-
# Base rollup globals for everything in the repo.
10+
# Base rollup globals for everything in the repo. Note that we want to disable
11+
# sorting of the globals as we manually group dict entries.
12+
# buildifier: disable=unsorted-dict-items
1113
ROLLUP_GLOBALS = {
14+
# Framework packages.
15+
"@angular/animations": "ng.animations",
16+
"@angular/common": "ng.common",
17+
"@angular/common/http": "ng.common.http",
18+
"@angular/common/http/testing": "ng.common.http.testing",
19+
"@angular/common/testing": "ng.common.testing",
20+
"@angular/core": "ng.core",
21+
"@angular/core/testing": "ng.core.testing",
22+
"@angular/forms": "ng.forms",
23+
"@angular/platform-browser": "ng.platformBrowser",
24+
"@angular/platform-browser-dynamic": "ng.platformBrowserDynamic",
25+
"@angular/platform-browser-dynamic/testing": "ng.platformBrowserDynamic.testing",
26+
"@angular/platform-browser/animations": "ng.platformBrowser.animations",
27+
"@angular/platform-server": "ng.platformServer",
28+
"@angular/router": "ng.router",
29+
30+
# Primary entry-points in the project.
1231
"@angular/cdk": "ng.cdk",
1332
"@angular/cdk-experimental": "ng.cdkExperimental",
1433
"@angular/google-maps": "ng.googleMaps",
1534
"@angular/material": "ng.material",
1635
"@angular/material-experimental": "ng.materialExperimental",
36+
"@angular/material-moment-adapter": "ng.materialMomentAdapter",
1737
"@angular/youtube-player": "ng.youtubePlayer",
1838

1939
# MDC Web
@@ -47,7 +67,12 @@ ROLLUP_GLOBALS = {
4767
"@material/tab-scroller": "mdc.tabScroller",
4868
"@material/text-field": "mdc.textField",
4969
"@material/top-app-bar": "mdc.topAppBar",
70+
71+
# Third-party libraries.
5072
"moment": "moment",
73+
"protractor": "protractor",
74+
"rxjs": "rxjs",
75+
"rxjs/operators": "rxjs.operators",
5176
"tslib": "tslib",
5277
}
5378

scripts/check-rollup-globals.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/**
2+
* Script that goes through each source file that will be included in the
3+
* release output and checks if any module imports are not part of the
4+
* specified rollup globals file.
5+
*
6+
* This script is used to validate Bazel rollup globals. We use a genrule to
7+
* convert the Starlark rollup globals dict into a JSON file which then can
8+
* be passed to this script to ensure that the rollup globals are up-to-date.
9+
*/
10+
11+
import chalk from 'chalk';
12+
import {readFileSync} from 'fs';
13+
import * as minimatch from 'minimatch';
14+
import {join, relative} from 'path';
15+
import * as ts from 'typescript';
16+
17+
const projectRoot = join(__dirname, '../');
18+
const args = process.argv.slice(2);
19+
20+
if (args.length !== 1) {
21+
console.error(chalk.red('No rollup globals file has been specified.'));
22+
process.exit(1);
23+
}
24+
25+
const rollupGlobals = JSON.parse(readFileSync(args[0], 'utf8'));
26+
const configFile = ts.readJsonConfigFile(join(projectRoot, 'tsconfig.json'), ts.sys.readFile);
27+
const parsedConfig = ts.parseJsonSourceFileConfigFileContent(configFile, ts.sys, projectRoot);
28+
const filesToCheckGlob = [
29+
'src/**/!(*.spec).ts',
30+
'!src/+(a11y-demo|e2e-app|universal-app|dev-app)/**/*.ts',
31+
'!src/**/schematics/**/*.ts',
32+
];
33+
34+
const failures = new Map<string, string[]>();
35+
36+
parsedConfig.fileNames.forEach(fileName => {
37+
const relativeFileName = relative(projectRoot, fileName);
38+
if (!filesToCheckGlob.every(g => minimatch(relativeFileName, g))) {
39+
return;
40+
}
41+
42+
const sourceFile =
43+
ts.createSourceFile(fileName, readFileSync(fileName, 'utf8'), ts.ScriptTarget.Latest, true);
44+
45+
const visitNode = (node: ts.Node) => {
46+
if (ts.isImportDeclaration(node)) {
47+
// Parse out the module name. The first and last characters are the quote marks.
48+
const module = node.moduleSpecifier.getText().slice(1, -1);
49+
const isExternal = !module.startsWith('.') && !module.startsWith('/');
50+
51+
// Check whether the module is external and whether it's in our rollup globals.
52+
if (isExternal && !rollupGlobals[module]) {
53+
failures.set(fileName, (failures.get(fileName) || []).concat(module));
54+
}
55+
}
56+
57+
ts.forEachChild(node, visitNode);
58+
};
59+
60+
ts.forEachChild(sourceFile, visitNode);
61+
});
62+
63+
if (failures.size) {
64+
console.error(chalk.red(' ✘ Rollup globals are not up-to-date.'));
65+
console.error();
66+
67+
failures.forEach((missingGlobals, fileName) => {
68+
console.error(chalk.yellow(` ⮑ ${fileName}:`));
69+
missingGlobals.forEach(g => console.error(` - ${g}`));
70+
});
71+
} else {
72+
console.info(chalk.green(' ✓ Rollup globals are up-to-date.'));
73+
}

src/cdk/config.bzl

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,14 @@ CDK_ENTRYPOINTS = [
1717
"table",
1818
"text-field",
1919
"tree",
20-
21-
# NOTE: "testing" should not be listed here as it will be treated as its own
22-
# package that will be included manually in the "ng_package".
20+
"testing",
2321
]
2422

25-
# List of all entry-point targets of the Angular Material package.
26-
CDK_TARGETS = ["//src/cdk"] + ["//src/cdk/%s" % ep for ep in CDK_ENTRYPOINTS]
23+
# List of all entry-point targets of the Angular Material package. Note that
24+
# we do not want to include "testing" here as it will be treated as a standalone
25+
# sub-package of the "ng_package".
26+
CDK_TARGETS = ["//src/cdk"] + \
27+
["//src/cdk/%s" % ep for ep in CDK_ENTRYPOINTS if not ep == "testing"]
2728

2829
# Within the CDK, only a few targets have sass libraries which need to be
2930
# part of the release package. This list declares all CDK targets with sass

src/material-experimental/mdc-button/testing/button-harness.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import {ComponentHarness, HarnessPredicate} from '@angular/cdk/testing';
1010
import {coerceBooleanProperty} from '@angular/cdk/coercion';
11-
import {ButtonHarnessFilters} from '@angular/material/button/testing/button-harness-filters';
11+
import {ButtonHarnessFilters} from '@angular/material/button/testing';
1212

1313

1414
/**

src/material-experimental/mdc-menu/testing/menu-harness.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import {ComponentHarness, HarnessPredicate} from '@angular/cdk/testing';
1010
import {coerceBooleanProperty} from '@angular/cdk/coercion';
1111
import {MatMenuItemHarness} from './menu-item-harness';
12-
import {MenuHarnessFilters} from '@angular/material/menu/testing/menu-harness-filters';
12+
import {MenuHarnessFilters} from '@angular/material/menu/testing';
1313

1414
/**
1515
* Harness for interacting with a MDC-based mat-menu in tests.

src/material-experimental/mdc-menu/testing/menu-item-harness.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import {ComponentHarness, HarnessPredicate} from '@angular/cdk/testing';
1010
import {coerceBooleanProperty} from '@angular/cdk/coercion';
11-
import {MenuItemHarnessFilters} from '@angular/material/menu/testing/menu-harness-filters';
11+
import {MenuItemHarnessFilters} from '@angular/material/menu/testing';
1212

1313

1414
/**

src/material/menu/testing/menu-item-harness.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import {ComponentHarness, HarnessPredicate} from '@angular/cdk/testing';
1010
import {coerceBooleanProperty} from '@angular/cdk/coercion';
11-
import {MenuItemHarnessFilters} from '@angular/material/menu/testing/menu-harness-filters';
11+
import {MenuItemHarnessFilters} from './menu-harness-filters';
1212

1313

1414
/**

src/material/radio/testing/radio-harness.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {ComponentHarness, HarnessPredicate} from '@angular/cdk/testing';
109
import {coerceBooleanProperty} from '@angular/cdk/coercion';
11-
import {RadioButtonHarnessFilters, RadioGroupHarnessFilters} from '@angular/material/radio/testing/radio-harness-filters';
10+
import {ComponentHarness, HarnessPredicate} from '@angular/cdk/testing';
11+
import {RadioButtonHarnessFilters, RadioGroupHarnessFilters} from './radio-harness-filters';
1212

1313
/**
1414
* Harness for interacting with a standard mat-radio-group in tests.

tools/package-tools/rollup-globals.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,31 @@ export const rollupGlobals = {
134134
...rollupYouTubePlayerEntryPoints,
135135
...rollupGoogleMapsEntryPoints,
136136

137+
// For each Angular Material secondary entry-point, we include a testing
138+
// tertiary entry-point.
139+
...matSecondaryEntryPoints.reduce(
140+
(res, entryPoint) => {
141+
return {...res, ...generateRollupEntryPoints(`material/${entryPoint}`, ['testing'])};
142+
},
143+
[]),
144+
145+
// For each Angular Material experimental secondary entry-point, we include a testing
146+
// tertiary entry-point.
147+
...materialExperimentalSecondaryEntryPoints.reduce(
148+
(res, entryPoint) => {
149+
return {
150+
...res,
151+
...generateRollupEntryPoints(`material-experimental/${entryPoint}`, ['testing'])
152+
};
153+
},
154+
[]),
155+
156+
// Manual entry-points which are not detected automatically. Since we do
157+
// not use this file for rollup-globals anymore, just as a backup if the Bazel
158+
// migrations does not work out, we can add these entries manually for now.
159+
'@angular/material-experimental/form-field/testing/control':
160+
'ng.materialExperimental.formField.testing.control',
161+
137162
'@angular/cdk/private/testing': 'ng.cdk.private.testing',
138163
'@angular/cdk/private/testing/e2e': 'ng.cdk.private.testing.e2e',
139164

tools/tslint-rules/missingRollupGlobalsRule.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class Walker extends Lint.RuleWalker {
3838

3939
this._configPath = path.resolve(process.cwd(), configPath);
4040
this._config = require(this._configPath).rollupGlobals;
41-
this._enabled = fileGlobs.some(p => minimatch(relativeFilePath, p));
41+
this._enabled = fileGlobs.every(p => minimatch(relativeFilePath, p));
4242
}
4343

4444
visitImportDeclaration(node: ts.ImportDeclaration) {

tslint.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,9 @@
134134
"missing-rollup-globals": [
135135
true,
136136
"./tools/package-tools/rollup-globals.ts",
137-
"src/!(a11y-demo|e2e-app|material-examples|universal-app|dev-app)/!(schematics)**/*.ts"
137+
"src/**/!(*.spec).ts",
138+
"!src/+(a11y-demo|e2e-app|universal-app|dev-app)/**/*.ts",
139+
"!src/**/schematics/**/*.ts"
138140
],
139141
"file-name-casing": [true, {
140142
// Exclude custom lint rule files since they have to always be camel-cased and end

0 commit comments

Comments
 (0)