Optimize computed properties output (byte-wise)#6652
Optimize computed properties output (byte-wise)#6652nicolo-ribaudo merged 3 commits intobabel:mainfrom
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/55254/ |
| var noo = "noo"; | ||
| var foo = "foo"; | ||
|
|
||
| var obj = { |
There was a problem hiding this comment.
I don't understand how it worked before, either foo and bar were undefined in the score or you now re-define them?
There was a problem hiding this comment.
This was purely a fixture/snapshot test, I've added also exec.js and that's why I had to add those to the scope in actual/expected fixtures (like you have noticed - they were previously undefined).
|
@Andarist do we stil want this change? |
|
I would be in favour of merging this (after rebase ofc), but unfortunately noone (except you) has reviewed this. |
|
My review wasn't very useful, to push this forward we could discuss about it during our next meeting. Could you please add it to our agenda here babel/notes#73. thanks |
loganfsmyth
left a comment
There was a problem hiding this comment.
Do you know what the actual byte difference ends up being once GZip is taken into account?
Is it worth exploring a new helper to optimize this instead? I don't know if that would be harder to keep the property evaluation ordering.
| var heh = "heh"; | ||
| var noo = "noo"; | ||
| var foo = "foo"; | ||
| var obj = babelHelpers.defineProperty(babelHelpers.defineProperty(babelHelpers.defineProperty(babelHelpers.defineProperty(babelHelpers.defineProperty({}, "x" + heh, "heh"), "y" + noo, "noo"), foo, "foo1"), "foo", "foo2"), "bar", "bar"); |
There was a problem hiding this comment.
If we go with this, we should probably have a cap on the nesting here, or else we risk objects with lots of keys creating extremely deep AST structures.
07fd689 to
e2375b7
Compare
|
@JLHwung @liuxingbaoyu This is something that we can still do. I rebased this PR, and added a cap as suggested in #6652 (comment). |
e2375b7 to
ff49c4e
Compare
|
Landing this PR was not my 2023 bingo card… 😝 |
|
Life is full of surprises 😛 |
ff49c4e to
18c83f2
Compare
It is a small "optimization" of the computed properties output. The change basically wraps (if possible) previous element -
initPropsobject or previousdefinePropertycall when "pushing" newdefinePropertycalls.It leverages the fact that
definePropertyreturns the passed object, therefore in most cases we do not need to create temporary references and stuff.babel-plugin-transform-computed-properties/test/fixtures/spec/multiple illustrates this nicely.
Before
After
Not only it saves some bytes in the output, it is probably more friendly for the minifiers too.