Fix evaluation order with object spread#11412
Conversation
|
Can we limit this deoptimization to when |
nicolo-ribaudo
added
the
PR: Spec Compliance 👓
nicolo-ribaudo
changed the title
| z = { | ||
| x, | ||
| w: _objectSpread({}, y) | ||
| w: _objectSpread(_objectSpread({}), y) |
There was a problem hiding this comment.
This change seems unnecessary.
There was a problem hiding this comment.
Yeah, scope.isPure is not working as intended. What's the difference between scope.isPure and path.isPure?
There was a problem hiding this comment.
😵 My bad , scope.isPure is working.
...es/babel-plugin-proposal-object-rest-spread/test/fixtures/object-spread/expression/output.js
Outdated
Show resolved
Hide resolved
nicolo-ribaudo
left a comment
nicolo-ribaudo
left a comment
There was a problem hiding this comment.
Thanks!
I have left a comment for another bug, that we can fix in a separate PR.
|
|
||
| }, pureB, {}, pureC, {}), impureFunc(), {}, pureD, { | ||
| pureD | ||
| }); |
There was a problem hiding this comment.
I would expect this output to be
var output = { ...pureA, get foo() {}, get bar() {}, ...pureB, ...pureC, ...impureFunc(), ...pureD, pureD }
var output = _objectSpread(
_objectSpread(
{},
pureA,
{ get foo() {}, get bar() {} },
pureB,
{},
pureC,
{}
),
impureFunc(),
{},
pureD,
{ pureD }
);I think that it's a bug in the isPure implementation, that only handles isClassMethod instead of isMethod (that also catches object methods/accessors).
There was a problem hiding this comment.
🙆♂️ I'm gonna give it a try.
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/21217/ |
When reviewing babel#11412, I noticed there are more cases where the intermediate values can mutate the spread reference. It's best demonstrated in the [TypeScript](https://github.com/microsoft/TypeScript/blob/eb105efdcd6db8a73f5b983bf329cb7a5eee55e1/src/compiler/transformers/es2018.ts#L272) examples.
github-actions
bot
added
the
outdated
4 participants
TLDR: currently, the code below is not transpiled correctly. REPL
expected
o.ato be 1.Long story: I found weird
__assignjs code generated bytsc. Out of curiosity, I checked the relevant transformer code and feed the test to babel 💥. This fix is migrated from TypeScript. Original Issue