Bug in build-optimizer or uglify-es (AOT output bug)

[fbernhard]

As per @trotyl on angular/angular#25800 I post this here:

Bug Report or Feature Request (mark with an x)

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

Command (mark with an x)

- [ ] new
- [ X] build
- [ ] serve
- [ ] test
- [ ] e2e
- [ ] generate
- [ ] add
- [ ] update
- [ ] lint
- [ ] xi18n
- [ ] run
- [ ] config
- [ ] help
- [ ] version
- [ ] doc

Versions

Angular CLI: 6.0.8
Node: 10.4.0
OS: darwin x64
Angular: 6.1.6

Repro steps

Current behavior

The following Typescript code:

let table1 = [null, "hello"];
let table2 = [null, "world"];

let i = -1;
for (const row of table1) {
  i++;
  if (!row) {
    continue;
  }
  console.log(row, table2[i])
}

when compiled with AOT in an Angular project (ng build --prod --build-optimizer --vendor-chunk=true --aot), produces the following "compiled" code:

for (var l = [null, "world"], n = -1, u = 0, a = [null, "hello"]; u < a.length; u++) {
   var o = a[u];
   o && console.log(o, l[++n])
}

The compiled code checks for null first before it increments i, the written Typescript codes increments i before the null check.

When run in a browser the AOT code prints hello null

Expected behavior

The AOT compiled code in a browser should print hello world

Minimal reproduction of the problem with instructions

Copy above Typescript snipped into a newly created Angular project and build it with AOT.

What is the motivation / use case for changing the behavior?

We expect AOT compiled code to do exactly what the code intends to do

[alan-agius4]

I'm sorry, but this issue is not caused by Angular CLI. Please contact the author(s) of the uglify-es project or file an issue on their issue tracker.

I have tried this directly using https://skalman.github.io/UglifyJS-online/ and I can re-produce the problem

[fbernhard]

@alan-agius4

I disagree:

The online tool that you pasted above produces a correct output:

for(var table1=[null,"hello"],table2=[null,"world"],i=-1,_i=0,table1_1=table1;_i<table1_1.length;_i++){var row=table1_1[_i];i++,row&&console.log(row,table2[i])}

--> hello world

Angular CLI produces a wrong output:

for (var l = [null, "world"], n = -1, u = 0, a = [null, "hello"]; u < a.length; u++) {
   var o = a[u];
   o && console.log(o, l[++n])
}

--> hello null

[alan-agius4]

@fbernhard sorry about that but I didn't realize I had some different uglify-es options.

The flag that is breaking your code is pure_getters, which in Angular CLI is set to true when using build optimizer in order to reduce the final bundle size.

Note in the latest beta version of the angular-cli, uglify-es was replaced with terser, maybe you can give it a shot and try if the problem persists?

[fbernhard]

Thank you for re-opening the issue. These type of changes are very dangerous for your users; We have more than 100k lines of code since Angular 2. We don't see these type of issues in tests as the they only appear in the AOT compilation. We have to "read" the code to find out where else this problem might occur.

…

On Tue, Sep 4, 2018, 19:04 Alan Agius ***@***.***> wrote: Reopened #12082 <#12082>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12082 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACDatKGi2lbfVD7UDP4LKg3-251ET9dQks5uXrKUgaJpZM4WY4kV> .

[michaelbaur]

I guess this is the same problem with pure_getters: true that is already discussed in issue 11439

[clydin]

This is actually a defect in uglify-es itself. terser appears to work properly with the option enabled and it will also be the default minifier in 7.0. uglify-es is unfortunately, no longer maintained.

[filipesilva]

Fixed via #11996, which will be the default in CLI 7.0.

[angular-automatic-lock-bot]

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.}