ng serve --prod with buildOptimizer turn on reports "Uncaught TypeError: Assignment to constant variable."

[kingfolk]

🐞 Bug report

Command (mark with an x)

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

Is this a regression?

It looks like a regression.
#9495
#12112

Description

reproduction repo: https://github.com/kingfolk/ng8-antlr4ts

ng new myApp
cd myApp
yarn add antlr4ts
ng serve --prod

will give

Uncaught TypeError: Assignment to constant variable.

The error will be gone if buildOptimizer is off. It seems like buildOptimizer is not correctly optimizing the code.

🌍 Your Environment

Angular CLI: 8.0.4
Node: 10.16.0
OS: darwin x64
Angular: 8.0.3
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.800.4
@angular-devkit/build-angular     0.800.4
@angular-devkit/build-optimizer   0.800.4
@angular-devkit/build-webpack     0.800.4
@angular-devkit/core              8.0.4
@angular-devkit/schematics        8.0.4
@angular/cli                      8.0.4
@ngtools/webpack                  8.0.4
@schematics/angular               8.0.4
@schematics/update                0.800.4
rxjs                              6.4.0
typescript                        3.4.5
webpack                           4.30.0

original issue: tunnelvisionlabs/antlr4ts#416

[kzc]

Looks like this happens when the name of an exported class outside of a namespace matches a namespace name and that namespace happens to export a variable. The name collision in typescript's generated code is harmless in this case as it would set a static property on the class, but I'm not sure whether this is a documented expected behavior of the typescript compiler.

If the local node_modules install of build-optimizer is patched to use let instead of const it'll probably be an adequate workaround for this issue:

--- a/node_modules/@angular-devkit/build-optimizer/src/transforms/wrap-enums.js	2019-06-28 19:10:40.000000000 -0400
+++ b/node_modules/@angular-devkit/build-optimizer/src/transforms/wrap-enums.js	2019-06-28 19:10:54.000000000 -0400
@@ -454,7 +454,7 @@
     const newStatement = [];
     newStatement.push(ts.createVariableStatement(isDefault ? undefined : modifiers, ts.createVariableDeclarationList([
         ts.createVariableDeclaration(name, undefined, pureIife),
-    ], ts.NodeFlags.Const)));
+    ], ts.NodeFlags.Let)));
     if (isDefault) {
         newStatement.push(ts.createExportAssignment(undefined, undefined, false, ts.createIdentifier(name)));
     }

I speculate that changing the original source code to have the namespace also wrap the class of the same name would be another workaround. Unverified.

[clydin]

Noticed that as well. The class IIFE wrapping should indeed use let as a class can be re-assigned.
The use of the namespace is also an odd construct in this instance.
A static property on the class would be less code and far more idiomatic; with the bonus of not triggering this issue.

[kzc]

Using static class properties in the original sources might also be advantageous with regard to dead code elimination. The code that typescript generates for the colliding export class/export namespace pattern could potentially impede tree shaking of a number of classes. Although it might be rendered moot in some cases by this library's use of "sideEffects": false.

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