Skip to content

Build optimizer leads to the broken build with AOT and ES2015 #8505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
devoto13 opened this issue Nov 15, 2017 · 11 comments · Fixed by #8689
Closed

Build optimizer leads to the broken build with AOT and ES2015 #8505

devoto13 opened this issue Nov 15, 2017 · 11 comments · Fixed by #8689
Assignees
Labels
P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful type: bug/fix

Comments

@devoto13
Copy link
Contributor

Bug Report or Feature Request (mark with an x)

- [x] bug report -> please search issues before submitting
- [ ] feature request

Versions.

Angular CLI: 1.5.0
Node: 8.7.0
OS: darwin x64
Angular: 5.0.1
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router

@angular/cli: 1.5.0
@angular-devkit/build-optimizer: 0.0.33
@angular-devkit/core: 0.0.20
@angular-devkit/schematics: 0.0.35
@ngtools/json-schema: 1.1.0
@ngtools/webpack: 1.8.0
@schematics/angular: 0.1.3
typescript: 2.4.2
webpack: 3.8.1

Repro steps.

npm i -g @angular/[email protected]
ng new esnext
cd esnext
# Set tsconfig.json target value as "es2015"
ng serve --prod
# Open browser and observe error
ng serve --prod --build-optimizer=false
# Open browser and observe working application

The log given by the failure.

main.3e29c45be9527f2aa1bf.bundle.js:1 ERROR TypeError: Cannot read property '_moduleDoBootstrap' of undefined
    at e.donePromise.then (main.3e29c45be9527f2aa1bf.bundle.js:1)
    at e.invoke (polyfills.ad37cd45a71cb38eee76.bundle.js:1)
    at Object.onInvoke (main.3e29c45be9527f2aa1bf.bundle.js:1)
    at e.invoke (polyfills.ad37cd45a71cb38eee76.bundle.js:1)
    at r.run (polyfills.ad37cd45a71cb38eee76.bundle.js:1)
    at polyfills.ad37cd45a71cb38eee76.bundle.js:1
    at e.invokeTask (polyfills.ad37cd45a71cb38eee76.bundle.js:1)
    at Object.onInvokeTask (main.3e29c45be9527f2aa1bf.bundle.js:1)
    at e.invokeTask (polyfills.ad37cd45a71cb38eee76.bundle.js:1)
    at r.runTask (polyfills.ad37cd45a71cb38eee76.bundle.js:1)

Desired functionality.

No error with build optimizer enabled.

Mention any other details that might be useful.

The issue was originally posted in #7797 (comment). I discovered that it is caused by build optimizer and not JIT mode, so creating separate issue for tracking purpose.

I traced it down back to this line. Changing amount of passed to 1 fixes the problem. It is probably a bug in UglifyJS after all, but I don't have enough expertise to trace it down and report correctly in their repo. Maybe somebody else can take it from here.

Relevant code snippet from exception:

bootstrapModuleFactory(e, t) {
                    const n = function(e) {
                        let t;
                        return t = "noop" === e ? new Lr : ("zone.js" === e ? void 0 : e) || new Dr({
                            enableLongStackTrace: Z()
                        })
                    }(t ? t.ngZone : void 0);
                    return n.run(()=>{
                        const t = jn.create([{
                            provide: Dr,
                            useValue: n
                        }], this.injector)
                          , r = e.create(t)
                          , o = r.injector.get(Wn, null);
                        if (!o)
                            throw new Error("No ErrorHandler. Is platform module (BrowserModule) included?");
                        return r.onDestroy(()=>K(this._modules, r)),
                        n.runOutsideAngular(()=>n.onError.subscribe({
                            next: e=>{
                                o.handleError(e)
                            }
                        })),
                        function(e, t, n) {
                            try {
                                const n = (()=>{
                                    const e = r.injector.get(dr);
                                    return e.runInitializers(),
exception here ->                       e.donePromise.then(()=>(this._moduleDoBootstrap(r),
                                    r))
                                }
                                )();
                                return R(n) ? n.catch(n=>{
                                    throw t.runOutsideAngular(()=>e.handleError(n)),
                                    n
                                }
                                ) : n
                            } catch (n) {
                                throw t.runOutsideAngular(()=>e.handleError(n)),
                                n
                            }
                        }(o, n)
                    }
                    )
                }
@squadwuschel
Copy link

got the same error only in Prod Build with aot and build optmizer on

@clydin
Copy link
Member

clydin commented Nov 16, 2017

It appears that uglify-es 3.1.9 has a bug in its new reduce_funcs compression option. Downgrading to 3.1.8 solves the problem and it looks like there are several fixes for the new option in place already in master so hopefully the next version addresses the problem.

@bsides
Copy link

bsides commented Nov 17, 2017

@clydin how did you test it with uglify-es 3.1.8? Manually adding into project's node_modules?

@kzc
Copy link

kzc commented Nov 20, 2017

uglify-es bug was isolated and reported: mishoo/UglifyJS#2496

@blub0hr
Copy link

blub0hr commented Nov 23, 2017

@clydin I tried upgrading to [email protected] in uglifyjs-webpack-plugin dependencies but still get the same error on a production build with --aot and --build-optimizer.

@filipesilva filipesilva self-assigned this Nov 24, 2017
@filipesilva filipesilva added blocked P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful type: bug/fix labels Nov 24, 2017
@filipesilva
Copy link
Contributor

@kzc thanks for keeping us posted on the bug status 👍

It looks like it was fixed in mishoo/UglifyJS#2504, which I suppose means it will be in the next release.

@kzc
Copy link

kzc commented Nov 24, 2017

@filipesilva Yes, uglify-js and uglify-es releases are generally made on the weekend after fuzz testing.

The transformed code in question was https://github.com/angular/angular/blob/5.0.1/packages/core/src/application_ref.ts#L219-L249

As you know that corresponds to the npm package @angular/[email protected] file node_modules/@angular/core/bundles/core.umd.js. When that file is run through

uglifyjs --toplevel -c passes=3,pure_getters --ecma 6 -b

with [email protected] and diffed with the latest uglify harmony branch (a.k.a. uglify-es) it produces:

$ diff -u 0.js 1.js
--- 0.js	2017-11-24 10:51:36.000000000 -0500
+++ 1.js	2017-11-24 10:51:12.000000000 -0500
@@ -3523,18 +3523,18 @@
                 }
             })), function(errorHandler, ngZone, callback) {
                 try {
-                    const result = (() => {
-                        const initStatus = moduleRef.injector.get(ApplicationInitStatus);
-                        return initStatus.runInitializers(), initStatus.donePromise.then(() => (this._moduleDoBootstrap(moduleRef), 
-                        moduleRef));
-                    })();
+                    const result = callback();
                     return isPromise(result) ? result.catch(e => {
                         throw ngZone.runOutsideAngular(() => errorHandler.handleError(e)), e;
                     }) : result;
                 } catch (e) {
                     throw ngZone.runOutsideAngular(() => errorHandler.handleError(e)), e;
                 }
-            }(exceptionHandler, ngZone);
+            }(exceptionHandler, ngZone, () => {
+                const initStatus = moduleRef.injector.get(ApplicationInitStatus);
+                return initStatus.runInitializers(), initStatus.donePromise.then(() => (this._moduleDoBootstrap(moduleRef), 
+                moduleRef));
+            });
         });
     }
     bootstrapModule(moduleType, compilerOptions = []) {

which I think is now in the correct scope.

@kzc
Copy link

kzc commented Nov 26, 2017

[email protected] was released. Please test and report your findings.

@filipesilva
Copy link
Contributor

Hi all, please see #8571 (comment) for instructions on how you can force update to [email protected].

@devoto13
Copy link
Contributor Author

I can confirm that [email protected] fixes the error.

PS For Yarn users the simple way to update only necessary packages is to remove record for uglify-es from yarn.lock file and run $ yarn install.

Example of uglify-es record (to be removed completely):


uglify-es@^3.1.3:
  version "3.1.9"
  resolved "https://registry.yarnpkg.com/uglify-es/-/uglify-es-3.1.9.tgz#6c82df628ac9eb7af9c61fd70c744a084abe6161"
  dependencies:
    commander "~2.11.0"
    source-map "~0.6.1"

filipesilva added a commit to filipesilva/angular-cli that referenced this issue Nov 30, 2017
filipesilva added a commit to filipesilva/angular-cli that referenced this issue Dec 1, 2017
clydin pushed a commit that referenced this issue Dec 1, 2017
clydin pushed a commit that referenced this issue Dec 4, 2017
dond2clouds pushed a commit to d2clouds/speedray-cli that referenced this issue Apr 23, 2018
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful type: bug/fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants