-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Replace var with const/let in library_(html5_)webgpu.js #21488
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The funny thing about using these newer constructs is that they can come with downsides:
- At runtime IIUC
var
can be a little faster sincelet
andconst
involve tracking some kind of dead zone information. - They can make the JS larger because
const
is a longer string thatvar
- They prohibit closure from re-writeing all
var
declaration into a single one per function (sincelet
/const
inside blocks are not supposed to be visible outside block).
None of these are a huge deal.. but its why we haven't made efforts to convert over yet.
CC @syg
interesting, I had guessed that Closure's analysis would take care of problems like that. A transform must be required for targeting ES5 so I kind of assumed that they would just do it unconditionally, instead of going through the work of preserving let/const when the target is ES6+. Since they do variable renaming, they already know what all the variables resolve to so I'd think they can do it without causing any observable behavior difference. This comment does seem to agree with my suspicion, although it's 3.5 years old. google/closure-compiler#3680 (comment) |
21afb70
to
c6e73a1
Compare
It was my assumption that closure compiler would indeed do this, but last time I tried to do |
Sounds like it's time to finally add a WebGPU code size test. I'll try to look into that next week. |
Closing for now, filed as a bug in #22496. |
Needs rebase after #21454