From 3420d2923a18972a8bf13815c6e94efb071ca47c Mon Sep 17 00:00:00 2001 From: Alan Date: Fri, 9 Aug 2019 09:55:36 +0200 Subject: [PATCH] fix(bazel): disable treeshaking when generating FESM and UMD bundles (#32069) There has been a regression where enabling rollup treeshaking causes errors during runtime because it will drop const access which will always evaluate to true or false. However, such `const` in `@angular/core` cannot be dropped because their value is changed when NGCC is run on `@angular/core` VE ``` const SWITCH_IVY_ENABLED__POST_R3__ = true; const SWITCH_IVY_ENABLED__PRE_R3__ = false; const ivyEnabled = SWITCH_IVY_ENABLED__PRE_R3__; ``` Ivy (After NGCC) ``` const SWITCH_IVY_ENABLED__POST_R3__ = true; const SWITCH_IVY_ENABLED__PRE_R3__ = false; const ivyEnabled = SWITCH_IVY_ENABLED__POST_R3__; ``` FESM2015 ``` load(path) { /** @type {?} */ const legacyOfflineMode = this._compiler instanceof Compiler; return legacyOfflineMode ? this.loadFactory(path) : this.loadAndCompile(path); } ``` ESM2015 ``` load(path) { /** @type {?} */ const legacyOfflineMode = !ivyEnabled && this._compiler instanceof Compiler; return legacyOfflineMode ? this.loadFactory(path) : this.loadAndCompile(path); } ``` From the above we can see that `ivyEnabled ` is being treeshaken away when generating the FESM bundle which is causing runtime errors such as `Cannot find module './lazy/lazy.module.ngfactory'` since in Ivy we will always load the factories. PR Close #32069 --- packages/bazel/src/ng_package/ng_package.bzl | 12 +++++++----- packages/bazel/test/ng_package/core_package.spec.ts | 4 ++++ 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/bazel/src/ng_package/ng_package.bzl b/packages/bazel/src/ng_package/ng_package.bzl index c2f77e13c494a..99b557f764445 100644 --- a/packages/bazel/src/ng_package/ng_package.bzl +++ b/packages/bazel/src/ng_package/ng_package.bzl @@ -112,11 +112,13 @@ def _rollup(ctx, bundle_name, rollup_config, entry_point, inputs, js_output, for args.add("--amd.id", package_name) # After updating to build_bazel_rules_nodejs 0.27.0+, rollup has been updated to v1.3.1 - # which tree shakes @__PURE__ annotations by default. We turn this feature off - # for ng_package as Angular bundles contain these annotations and there are - # test failures if they are removed. See comments in - # https://github.com/angular/angular/pull/29210 for more information. - args.add("--no-treeshake.annotations") + # which tree shakes @__PURE__ annotations and const variables which are later ammended by NGCC. + # We turn this feature off for ng_package as Angular bundles contain these and there are + # test failures if they are removed. + # See comments in: + # https://github.com/angular/angular/pull/29210 + # https://github.com/angular/angular/pull/32069 + args.add("--no-treeshake") # Note: if the input has external source maps then we need to also install and use # `rollup-plugin-sourcemaps`, which will require us to use rollup.config.js file instead diff --git a/packages/bazel/test/ng_package/core_package.spec.ts b/packages/bazel/test/ng_package/core_package.spec.ts index 934ad3682cf80..81f68ce93e41d 100644 --- a/packages/bazel/test/ng_package/core_package.spec.ts +++ b/packages/bazel/test/ng_package/core_package.spec.ts @@ -133,6 +133,10 @@ describe('@angular/core ng_package', () => { } else { it('should have decorators', () => { expect(shx.cat('fesm5/core.js')).toContain('__decorate'); }); + + // See: https://github.com/angular/angular/pull/32069 + it('should retain access to const', + () => { expect(shx.cat('fesm5/core.js')).toContain('!ivyEnabled'); }); } it('should load tslib from external bundle', () => {