Skip to content

Commit 2ec7254

Browse files
authored
build: remove ngcc postinstall patch in favor of separate script (#18355)
Currently we always patch `ngcc` as part of a postinstall patch. This is because Ngcc does not overwrite the `main` field of processed packages. Though, since we want to run `nodejs_binary`/`nodejs_test` targets with Ivy, we need to either have custom module resolution, or just need to update the `package.json` files to point to the NGCC processed bundles. Implementing a custom module resolution for each of these target seems rather incovenient and less reliable. Instead, we just ensure the `package.json` files point to the right files. Currently we achieve this by patching ngcc to always update the `main` property. This patch is prone to upstream ngcc changes, so we move it into a separate script that just runs _after_ ngcc, and updates the `package.json` files. The benfit of this is that `ngcc` doesn't need to be patched, and upstream ngcc changes are not likely breaking the components repo unit test (as seen in: https://circleci.com/gh/angular/angular/608106).
1 parent 12edc0b commit 2ec7254

File tree

4 files changed

+45
-10
lines changed

4 files changed

+45
-10
lines changed

WORKSPACE

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,12 @@ node_repositories(
4949

5050
yarn_install(
5151
name = "npm",
52-
# We add the postinstall patches file here so that Yarn will rerun whenever
53-
# the patches script changes.
54-
data = ["//:tools/bazel/postinstall-patches.js"],
52+
# We add the postinstall patches file, and ngcc main fields update script here so
53+
# that Yarn will rerun whenever one of these files has been modified.
54+
data = [
55+
"//:tools/bazel/postinstall-patches.js",
56+
"//:tools/bazel/update-ngcc-main-fields.js",
57+
],
5558
package_json = "//:package.json",
5659
yarn_lock = "//:yarn.lock",
5760
)

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
"yarn": ">= 1.19.1"
1414
},
1515
"scripts": {
16-
"postinstall": "node tools/bazel/postinstall-patches.js && ngcc --properties main --create-ivy-entry-points",
16+
"postinstall": "node tools/bazel/postinstall-patches.js && ngcc --properties main --create-ivy-entry-points && node tools/bazel/update-ngcc-main-fields.js",
1717
"build": "node ./scripts/build-packages-dist.js",
1818
"bazel:buildifier": "find . -type f \\( -name \"*.bzl\" -or -name WORKSPACE -or -name BUILD -or -name BUILD.bazel \\) ! -path \"*/node_modules/*\" | xargs buildifier -v --warnings=attr-cfg,attr-license,attr-non-empty,attr-output-default,attr-single-file,constant-glob,ctx-args,depset-iteration,depset-union,dict-concatenation,duplicated-name,filetype,git-repository,http-archive,integer-division,load,load-on-top,native-build,native-package,output-group,package-name,package-on-top,redefined-variable,repository-name,same-origin-load,string-iteration,unused-variable,unsorted-dict-items,out-of-order-load",
1919
"bazel:format-lint": "yarn -s bazel:buildifier --lint=warn --mode=check",

tools/bazel/postinstall-patches.js

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,6 @@ searchAndReplace(
106106
// Workaround for: https://github.com/bazelbuild/rules_nodejs/issues/1208.
107107
applyPatch(path.join(__dirname, './manifest_externs_hermeticity.patch'));
108108

109-
// Workaround for using Ngcc with "--create-ivy-entry-points". This is a special
110-
// issue for our repository since we want to run Ivy by default in the module resolution,
111-
// but still have the option to opt-out by not using the compiled ngcc entry-points.
112-
searchAndReplace(`[formatProperty + "_ivy_ngcc"]`, '[formatProperty]',
113-
'node_modules/@angular/compiler-cli/ngcc/src/writing/new_entry_point_file_writer.js');
114-
115109
// Workaround for https://github.com/angular/angular/issues/33452:
116110
searchAndReplace(/angular_compiler_options = {/, `$&
117111
"strictTemplates": True,`, 'node_modules/@angular/bazel/src/ng_module.bzl');
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Script that runs after node modules have been installed, and Ngcc processed all packages.
3+
* This script updates the `package.json` files of Angular framework packages to point to the
4+
* Ngcc processed UMD bundles. This is needed because we run Angular in a `nodejs_binary`, but
5+
* want to make sure that Ivy is being used. By default, the NodeJS module resolution will load
6+
* the unprocessed UMD bundle because the `main` field of the `package.json` files point to the
7+
* View Engine UMD bundles. This script updates the `main` field in `package.json` files to point
8+
* to the previously generated Ivy UMD bundles.
9+
*
10+
* Ngcc does not by edit the `main` field because we ran it with the `--create-ivy-entry-points`
11+
* flag. It instructs Ngcc to not modify existing package bundles, but rather create separate
12+
* copies with the needed Ivy modifications. This is necessary because the original bundles
13+
* are needed for View Engine, and we want to preserve them in order to be able to switch
14+
* between Ivy and View Engine (for testing). Since the goal of this flag is to not modify
15+
* any original package files/bundles, Ngcc will not edit the `main` field to point to
16+
* the processed Ivy bundles.
17+
*/
18+
19+
const shelljs = require('shelljs');
20+
const fs = require('fs');
21+
22+
const MAIN_FIELD_NAME = 'main';
23+
const NGCC_MAIN_FIELD_NAME = 'main_ivy_ngcc';
24+
25+
shelljs.find('node_modules/@angular/**/package.json').forEach(filePath => {
26+
// Do not update `package.json` files for deeply nested node modules (e.g. dependencies of
27+
// the `@angular/compiler-cli` package).
28+
if (filePath.lastIndexOf('node_modules/') !== 0) {
29+
return;
30+
}
31+
const parsedJson = JSON.parse(fs.readFileSync(filePath, 'utf8'));
32+
if (parsedJson[NGCC_MAIN_FIELD_NAME] &&
33+
parsedJson[MAIN_FIELD_NAME] !== parsedJson[NGCC_MAIN_FIELD_NAME]) {
34+
// Update the main field to point to the ngcc main script.
35+
parsedJson[MAIN_FIELD_NAME] = parsedJson[NGCC_MAIN_FIELD_NAME];
36+
fs.writeFileSync(filePath, JSON.stringify(parsedJson, null, 2));
37+
}
38+
});

0 commit comments

Comments
 (0)