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

feat(es/minifier): mark some builtin functions pure. #6842

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

hyf0
Copy link
Contributor

@hyf0 hyf0 commented Jan 21, 2023

Description:

          Can we have those list also in swc minifier? (itself)

Originally posted by @kdy1 in #6840 (comment)

BREAKING CHANGE:

Related issue (if exists):

The PR relies on code in #6840.

member_expr!(DUMMY_SP, console.groupCollapsed),
member_expr!(DUMMY_SP, console.groupEnd),
member_expr!(DUMMY_SP, console.info),
member_expr!(DUMMY_SP, console.log),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about console. I often use console.log to debug or capture some variables to make the minifier/mangler not optimize it.

Copy link
Member

Choose a reason for hiding this comment

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

Same here 🤣

Copy link
Member

Choose a reason for hiding this comment

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

I used it while debugging the minifier

@kdy1 kdy1 self-assigned this Jan 25, 2023
if let Callee::Expr(e) = &n.callee {
// Check for pure_funcs
if pure_fns.contains(&HashEqIgnoreSpanExprRef(e)) {
if self.pure_funcs.contains(&HashEqIgnoreSpanExprRef(e)) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we use two hashset instead?
I think it will avoid allocations

Copy link
Member

Choose a reason for hiding this comment

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

One hashset with type Lazy<FxHashSet<HashEqIgnoreSpanExprRef<'static>>>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Get it.

@hyf0
Copy link
Contributor Author

hyf0 commented Jan 25, 2023

I think I misunderstand the usage of the esbuild's list.

Note that membership in this list says nothing about whether calling any of
these functions has any side effects. It only says something about
referencing these function without calling them (doesn't have side effects).

@hyf0
Copy link
Contributor Author

hyf0 commented Jan 25, 2023

But still, there are indeed some expressions in the list that are calling without side effects. I need to tweak the list.

@kdy1 kdy1 removed their assignment Apr 5, 2023
@kdy1 kdy1 added this to the Planned milestone Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants