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

🏗🚀 Mangle values of specific enum objects #36935

Merged
merged 5 commits into from
Nov 17, 2021

Conversation

alanorozco
Copy link
Member

@alanorozco alanorozco commented Nov 15, 2021

Reduces compressed size of amp-story bundles by ~0.6K

@alanorozco alanorozco marked this pull request as ready for review November 16, 2021 19:53
@amp-owners-bot
Copy link

amp-owners-bot bot commented Nov 16, 2021

Hey @erwinmombay! These files were changed:

build-system/babel-plugins/babel-plugin-mangle-object-values/index.js
build-system/babel-plugins/babel-plugin-mangle-object-values/test/fixtures/transform/mangle/input.js
build-system/babel-plugins/babel-plugin-mangle-object-values/test/fixtures/transform/mangle/output.js
build-system/babel-plugins/babel-plugin-mangle-object-values/test/fixtures/transform/options.json
build-system/babel-plugins/babel-plugin-mangle-object-values/test/index.js

Hey @jridgewell! These files were changed:

build-system/babel-plugins/babel-plugin-mangle-object-values/index.js
build-system/babel-plugins/babel-plugin-mangle-object-values/test/fixtures/transform/mangle/input.js
build-system/babel-plugins/babel-plugin-mangle-object-values/test/fixtures/transform/mangle/output.js
build-system/babel-plugins/babel-plugin-mangle-object-values/test/fixtures/transform/options.json
build-system/babel-plugins/babel-plugin-mangle-object-values/test/index.js
src/core/types/enum.js

Hey @gmajoulet, @newmuis! These files were changed:

extensions/amp-story/1.0/amp-story-store-service.js

extensions/amp-story/1.0/amp-story-store-service.js Outdated Show resolved Hide resolved
extensions/amp-story/1.0/amp-story-store-service.js Outdated Show resolved Hide resolved
if (!charset) {
// Letters first since they allow unbracketed syntax when used as keys:
// (foo.a0 vs. foo['0a'])
charset = indexCharset(
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional: If there are only 10 (or fewer) enum values, we could optimize to a number instead. That'd save a few bytes.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we use numbers let's please exclude 0 that's falsy

Copy link
Member Author

@alanorozco alanorozco Nov 16, 2021

Choose a reason for hiding this comment

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

From my local output, using numbers from 0..99 seems to reduce the amp-story bundle by a further 75 B compressed. Cool!

Note that I'm using letters only for values ≥ 99 since we could be using the enum values as keys (like StateProperty) and we'd like to prevent the collision of foo[0] vs. foo['0']

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