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

Lint enum usage so that they are fully DCE-able #36754

Open
jridgewell opened this issue Nov 4, 2021 · 3 comments
Open

Lint enum usage so that they are fully DCE-able #36754

jridgewell opened this issue Nov 4, 2021 · 3 comments
Assignees
Labels
Stale Inactive for one year or more Type: Feature Request

Comments

@jridgewell
Copy link
Contributor

jridgewell commented Nov 4, 2021

Description

Context: #36453

Currently, enum objects are left in the output binaries because we're using them as dynamic object values (eg, loop keys or performing dynamic Enum[key] lookups). In an ideal codebase, enums would be used via Enum.keyName lookups only, which means that the value associated with that key can be inlined directly in to output code.

We suspect this is one of the last issue with that causes Terser's outputs to be larger than Closure compiler.

@jridgewell jridgewell self-assigned this Nov 4, 2021
jridgewell added a commit to jridgewell/amphtml that referenced this issue Nov 4, 2021
This allows us to extend the exceptions and add `_Enum` as an approved case.

Partial for ampproject#36754
jridgewell added a commit to jridgewell/amphtml that referenced this issue Nov 4, 2021
This allows us to extend the exceptions and add `_Enum` as an approved case.

Partial for ampproject#36754
jridgewell added a commit to jridgewell/amphtml that referenced this issue Nov 4, 2021
jridgewell added a commit to jridgewell/amphtml that referenced this issue Nov 4, 2021
jridgewell added a commit to jridgewell/amphtml that referenced this issue Nov 4, 2021
This allows us to extend the exceptions and add `_Enum` as an approved case.

Partial for ampproject#36754
jridgewell added a commit to jridgewell/amphtml that referenced this issue Nov 4, 2021
jridgewell added a commit that referenced this issue Nov 4, 2021
* Switch from google-camelcase to local eslint plugin

This allows us to extend the exceptions and add `_Enum` as an approved case.

Partial for #36754

* Forgot to commit camelcase

* Fix const reassignment
jridgewell added a commit to jridgewell/amphtml that referenced this issue Nov 4, 2021
jridgewell added a commit to jridgewell/amphtml that referenced this issue Nov 5, 2021
jridgewell added a commit that referenced this issue Nov 5, 2021
* Add optional static enum lint

Partial for #36754

* Demonstrate with changes

* Exempt tests from enums linting

* Apply suggestions from code review
@samouri
Copy link
Member

samouri commented Nov 19, 2021

Can this be closed now that all enums have been converted to static and the lint rule has been made?

@jridgewell
Copy link
Contributor Author

We still need to convert extensions, and make the lint opt-out instead of opt-in.

@stale
Copy link

stale bot commented Nov 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale Inactive for one year or more label Nov 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Inactive for one year or more Type: Feature Request
Projects
None yet
Development

No branches or pull requests

2 participants