Skip to content

build-optimizer: enum wrapping pass is unsafe in atypical use cases #12160

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
alexeagle opened this issue Sep 5, 2018 · 3 comments
Closed

build-optimizer: enum wrapping pass is unsafe in atypical use cases #12160

alexeagle opened this issue Sep 5, 2018 · 3 comments
Labels
freq1: low Only reported by a handful of users who observe it rarely severity3: broken type: bug/fix workaround2: non-obvious
Milestone

Comments

@alexeagle
Copy link
Contributor

From @clydin on October 23, 2017 14:27

Bug Report or Feature Request (mark with an x)

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

Area

- [X] devkit
- [ ] schematics

Versions

Repro steps

An enum member can contain an initializer which may have side effects. Currently all enums are wrapping in an IIFE and annotated with PURE which will prevent the side effects from occurring if the enum is unused.

function getValue() {
  console.log('hi');
  return 6;
}

enum Test {
  ValueA,
  ValueB = getValue()
}

Desired functionality

As this will most likely be an extremely rare occurrence, a documentation note may suffice. Additionally a warning or informational message may also be useful when an enum is encountered with a member containing a non-constant expression.

Mention any other details that might be useful

Copied from original issue: angular/devkit#234

@OrangeDog
Copy link

@alexeagle looks like you typo'd the workaround label?

@dgp1130
Copy link
Collaborator

dgp1130 commented May 26, 2020

Just verified this locally and I'm still able to reproduce on v9.1.7. I didn't look into the pure comments for Terser, but running the following code:

function getBar(): number {
  console.log('Getting bar');
  return 2;
}

enum MyEnum {
  foo = 1,
  bar = getBar(),
}

Will print "Getting bar" when run with ng serve and will print nothing with ng serve --prod. Most likely the enum is getting tree shaken because it is unused and assumed to be pure via Terser comments when it actually isn't. I don't see any warning or compile-time error as a result of using a side-effectful function in an enum initializer.

clydin added a commit to clydin/angular-cli that referenced this issue May 27, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests of a production build of AIO resulted in a 15% total build time reduction.

Closes angular#12160
clydin added a commit to clydin/angular-cli that referenced this issue May 28, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests of a production build of AIO resulted in a 15% total build time reduction.

Closes angular#12160
clydin added a commit to clydin/angular-cli that referenced this issue May 28, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests of a production build of AIO resulted in a 15% total build time reduction.

Closes angular#12160
clydin added a commit to clydin/angular-cli that referenced this issue May 29, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests of a production build of AIO resulted in a 15% total build time reduction.

Closes angular#12160
clydin added a commit to clydin/angular-cli that referenced this issue May 30, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests (with caching disabled) of a production build of AIO resulted in a 10% total build time reduction.

Closes angular#12160
clydin added a commit to clydin/angular-cli that referenced this issue May 30, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests of a production build of AIO resulted in a ~10% total build time reduction.

Closes angular#12160
clydin added a commit to clydin/angular-cli that referenced this issue May 31, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests of a production build of AIO resulted in a ~10% total build time reduction.

Closes angular#12160
clydin added a commit to clydin/angular-cli that referenced this issue May 31, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests of a production build of AIO resulted in a ~10% total build time reduction.

Closes angular#12160
clydin added a commit to clydin/angular-cli that referenced this issue May 31, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests of a production build of AIO resulted in a ~10% total build time reduction.

Closes angular#12160
clydin added a commit to clydin/angular-cli that referenced this issue May 31, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests of a production build of AIO resulted in a ~10% total build time reduction.

Closes angular#12160
clydin added a commit to clydin/angular-cli that referenced this issue May 31, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests of a production build of AIO resulted in a ~10% total build time reduction.

Closes angular#12160
clydin added a commit to clydin/angular-cli that referenced this issue Jun 1, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests of a production build of AIO resulted in a ~10% total build time reduction.

Closes angular#12160
clydin added a commit to clydin/angular-cli that referenced this issue Jun 1, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests of a production build of AIO resulted in a ~10% total build time reduction.

Closes angular#12160
clydin added a commit to clydin/angular-cli that referenced this issue Jun 2, 2021
…es to babel plugins

The build optimizer passes that are still relevant for Ivy are now implemented as babel plugins.  Using babel has several advantages.  The `babel-loader` is already present within the build pipeline and many packages will already need to be processed by it.  The `babel-loader` also provides the necessary infrastructure to setup babel and link it to the Webpack build system.  This removes the need for the infrastructure code within the build optimizer loader and reduces the build optimizer to only a set of transformation plugins for babel to consume.  The babel plugin architecture also allows for less code to represent similar transformations and provides a variety of helper utilities which further reduces the amount code needed.
The passes are now implemented to be safer when transforming code.  Enum values which contain potential side effects are also no longer altered.  Enums are wrapped in less destructive manner which reduces the likelihood of incorrectly emitting the transformed enum. Class static fields which contain potential side effects are no longer wrapped in pure annotated IIFEs.  Known safe Angular static fields are also wrapped and Angular static fields which are not required in production code are also dropped.
The conversion of the passes not only reduces the amount of code to maintain but also provides a noticeable performance improvement.  Initial tests of a production build of AIO resulted in a ~10% total build time reduction.

Closes angular#12160
@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 Jul 5, 2021
@clydin clydin removed their assignment Jun 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
freq1: low Only reported by a handful of users who observe it rarely severity3: broken type: bug/fix workaround2: non-obvious
Projects
None yet
Development

No branches or pull requests

5 participants