From b6dfa7628d199e1754bc773bf0f289b77f538f3b Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 31 Jan 2020 17:49:43 +0100 Subject: [PATCH 1/2] build: remove ngcc postinstall patch in favor of separate script 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). --- WORKSPACE | 9 +++++--- package.json | 2 +- tools/bazel/postinstall-patches.js | 6 ------ tools/bazel/update-ngcc-main-fields.js | 29 ++++++++++++++++++++++++++ 4 files changed, 36 insertions(+), 10 deletions(-) create mode 100644 tools/bazel/update-ngcc-main-fields.js diff --git a/WORKSPACE b/WORKSPACE index af4ff7fff2d9..2d6eff4ffd0a 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -49,9 +49,12 @@ node_repositories( yarn_install( name = "npm", - # We add the postinstall patches file here so that Yarn will rerun whenever - # the patches script changes. - data = ["//:tools/bazel/postinstall-patches.js"], + # We add the postinstall patches file, and ngcc main fields update script here so + # that Yarn will rerun whenever one of these files has been modified. + data = [ + "//:tools/bazel/postinstall-patches.js", + "//:tools/bazel/update-ngcc-main-fields.js", + ], package_json = "//:package.json", yarn_lock = "//:yarn.lock", ) diff --git a/package.json b/package.json index ccded92525cf..e52f58cb6c78 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "yarn": ">= 1.19.1" }, "scripts": { - "postinstall": "node tools/bazel/postinstall-patches.js && ngcc --properties main --create-ivy-entry-points", + "postinstall": "node tools/bazel/postinstall-patches.js && ngcc --properties main --create-ivy-entry-points && node tools/bazel/update-ngcc-main-fields.js", "build": "node ./scripts/build-packages-dist.js", "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", "bazel:format-lint": "yarn -s bazel:buildifier --lint=warn --mode=check", diff --git a/tools/bazel/postinstall-patches.js b/tools/bazel/postinstall-patches.js index 77de9da082eb..fe571c9c4caa 100644 --- a/tools/bazel/postinstall-patches.js +++ b/tools/bazel/postinstall-patches.js @@ -106,12 +106,6 @@ searchAndReplace( // Workaround for: https://github.com/bazelbuild/rules_nodejs/issues/1208. applyPatch(path.join(__dirname, './manifest_externs_hermeticity.patch')); -// Workaround for using Ngcc with "--create-ivy-entry-points". This is a special -// issue for our repository since we want to run Ivy by default in the module resolution, -// but still have the option to opt-out by not using the compiled ngcc entry-points. -searchAndReplace(`[formatProperty + "_ivy_ngcc"]`, '[formatProperty]', - 'node_modules/@angular/compiler-cli/ngcc/src/writing/new_entry_point_file_writer.js'); - // Workaround for https://github.com/angular/angular/issues/33452: searchAndReplace(/angular_compiler_options = {/, `$& "strictTemplates": True,`, 'node_modules/@angular/bazel/src/ng_module.bzl'); diff --git a/tools/bazel/update-ngcc-main-fields.js b/tools/bazel/update-ngcc-main-fields.js new file mode 100644 index 000000000000..943b1033fbbb --- /dev/null +++ b/tools/bazel/update-ngcc-main-fields.js @@ -0,0 +1,29 @@ +/** + * Script that runs after node modules have been installed, and NGCC processed all packages. + * This script updates the `package.json` files of `@angular` framework packages to point + * to the NGCC processed UMD bundles. This is needed because we run Angular in a `nodejs_binary`, + * but want to make sure that Ivy is being used. By default, the NodeJS module resolution will + * load the unprocessed UMD bundle, so we update the `main` field in `package.json` files to point + * to the Ivy UMD bundles. + */ + +const shelljs = require('shelljs'); +const fs = require('fs'); + +const MAIN_FIELD_NAME = 'main'; +const NGCC_MAIN_FIELD_NAME = 'main_ivy_ngcc'; + +shelljs.find('node_modules/@angular/**/package.json').forEach(filePath => { + // Do not update `package.json` files for deeply nested node modules (e.g. dependencies of + // the `@angular/compiler-cli` package). + if (filePath.lastIndexOf('node_modules/') !== 0) { + return; + } + const parsedJson = JSON.parse(fs.readFileSync(filePath, 'utf8')); + if (parsedJson[NGCC_MAIN_FIELD_NAME] && + parsedJson[MAIN_FIELD_NAME] !== parsedJson[NGCC_MAIN_FIELD_NAME]) { + // Update the main field to point to the ngcc main script. + parsedJson[MAIN_FIELD_NAME] = parsedJson[NGCC_MAIN_FIELD_NAME]; + fs.writeFileSync(filePath, JSON.stringify(parsedJson, null, 2)); + } +}); From 34f4d915668b959234d19d2358dc71de8cd11073 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sat, 1 Feb 2020 19:24:02 +0100 Subject: [PATCH 2/2] fixup! build: remove ngcc postinstall patch in favor of separate script Address feedback --- tools/bazel/update-ngcc-main-fields.js | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tools/bazel/update-ngcc-main-fields.js b/tools/bazel/update-ngcc-main-fields.js index 943b1033fbbb..f31de48769e7 100644 --- a/tools/bazel/update-ngcc-main-fields.js +++ b/tools/bazel/update-ngcc-main-fields.js @@ -1,10 +1,19 @@ /** - * Script that runs after node modules have been installed, and NGCC processed all packages. - * This script updates the `package.json` files of `@angular` framework packages to point - * to the NGCC processed UMD bundles. This is needed because we run Angular in a `nodejs_binary`, - * but want to make sure that Ivy is being used. By default, the NodeJS module resolution will - * load the unprocessed UMD bundle, so we update the `main` field in `package.json` files to point - * to the Ivy UMD bundles. + * Script that runs after node modules have been installed, and Ngcc processed all packages. + * This script updates the `package.json` files of Angular framework packages to point to the + * Ngcc processed UMD bundles. This is needed because we run Angular in a `nodejs_binary`, but + * want to make sure that Ivy is being used. By default, the NodeJS module resolution will load + * the unprocessed UMD bundle because the `main` field of the `package.json` files point to the + * View Engine UMD bundles. This script updates the `main` field in `package.json` files to point + * to the previously generated Ivy UMD bundles. + * + * Ngcc does not by edit the `main` field because we ran it with the `--create-ivy-entry-points` + * flag. It instructs Ngcc to not modify existing package bundles, but rather create separate + * copies with the needed Ivy modifications. This is necessary because the original bundles + * are needed for View Engine, and we want to preserve them in order to be able to switch + * between Ivy and View Engine (for testing). Since the goal of this flag is to not modify + * any original package files/bundles, Ngcc will not edit the `main` field to point to + * the processed Ivy bundles. */ const shelljs = require('shelljs');