Skip to content
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

add babel-plugin-transform-block-scoping #30807

Merged
merged 3 commits into from
Oct 22, 2020

Conversation

erwinmombay
Copy link
Member

@erwinmombay erwinmombay commented Oct 21, 2020

A discussion in evanw/esbuild#478 has shown that using let and class in the same enclosing/block level has a significant performance impact on the Safari browser when there is a large number of instances of the block scoped variables to track (such as a large bundle with a large number of let/const/class declarations). To alleviate this problem we transform the let and class constructs into var when it is 'easily' possible (ex. we bail on the for loop block scoping to not complicate the transform).

this should only be turned on for esm/sxg in post closure transforms

We do not do this transform to const as we do a const to let transformer prior to this transform. As a follow up PR i will combine this with the let to const transformer

Co-authored-by: Justin Ridgewell jridgewell@google.com
Co-authored-by: erwin mombay erwinm@google.com

A discussion in evanw/esbuild#478 has shown
that using let and class in same enclosing level has a signigicant
performance impact on the Safari browser. To alleviate this problem we
transform the let and class constructs into var when it is 'easily'
possible.

Co-authored-by: Justin Ridgewell <jridgewell@google.com>
Co-authored-by: erwin mombay <erwinm@google.com>
@amp-owners-bot
Copy link

Hey @jridgewell! These files were changed:

build-system/babel-plugins/babel-plugin-transform-block-scoping/index.js
build-system/babel-plugins/babel-plugin-transform-block-scoping/test/fixtures/transform-class-declaration/transform-class/input.js
build-system/babel-plugins/babel-plugin-transform-block-scoping/test/fixtures/transform-class-declaration/transform-class/options.json
build-system/babel-plugins/babel-plugin-transform-block-scoping/test/fixtures/transform-class-declaration/transform-class/output.js
build-system/babel-plugins/babel-plugin-transform-block-scoping/test/fixtures/transform-variable-declaration/no-transform-const/input.js
build-system/babel-plugins/babel-plugin-transform-block-scoping/test/fixtures/transform-variable-declaration/no-transform-const/options.json
build-system/babel-plugins/babel-plugin-transform-block-scoping/test/fixtures/transform-variable-declaration/no-transform-const/output.js
build-system/babel-plugins/babel-plugin-transform-block-scoping/test/fixtures/transform-variable-declaration/transform-let/input.js
build-system/babel-plugins/babel-plugin-transform-block-scoping/test/fixtures/transform-variable-declaration/transform-let/options.json
build-system/babel-plugins/babel-plugin-transform-block-scoping/test/fixtures/transform-variable-declaration/transform-let/output.js
build-system/babel-plugins/babel-plugin-transform-block-scoping/test/index.js

@kristoferbaxter
Copy link
Contributor

Agree with @jridgewell feedback that we'll need to turn on mangle here now (https://github.com/ampproject/amphtml/blob/master/build-system/compile/post-closure-babel.js#L33).

Copy link
Contributor

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mangle will be needed to rename the variable references.

@erwinmombay erwinmombay merged commit 9799f73 into ampproject:master Oct 22, 2020
@erwinmombay
Copy link
Member Author

size before this change:
Computing bundle size for version 2010212121000 at commit 6418ea9.
brotli bundle sizes are:
dist/v0.mjs: 59.49KB

size after this change:
Computing bundle size for version 2010220007000 at commit 9799f73.
brotli bundle sizes are:
dist/v0.mjs: 59.48KB

jridgewell added a commit that referenced this pull request Nov 16, 2020
ed-bird pushed a commit to ed-bird/amphtml that referenced this pull request Dec 10, 2020
* add babel-plugin-transform-block-scoping

A discussion in evanw/esbuild#478 has shown
that using let and class in same enclosing level has a signigicant
performance impact on the Safari browser. To alleviate this problem we
transform the let and class constructs into var when it is 'easily'
possible.

Co-authored-by: Justin Ridgewell <jridgewell@google.com>
Co-authored-by: erwin mombay <erwinm@google.com>

* turn on mangle on terser

* Comments and test cases

Co-authored-by: Justin Ridgewell <jridgewell@google.com>
jridgewell added a commit that referenced this pull request Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants