Node: object spread operator mutates first argument

[larsthorup]

• Version: v11.4.0
• Platform: Windows 10.0.17134, 64-bit
• Subsystem: don't know

Code to reproduce the issue:

const workspace = JSON.parse('{"orgContentScore":{"41":{"id":"41","contentId":"111","competenceId":"40","scoreTypeId":"6","value":0.25}}}');
const selectedObject = Object.values(workspace.orgContentScore).filter(ocs => ocs.contentId === '111').find(ocs => ocs.competenceId === '40');
console.log(selectedObject.value, 0.25); // Note: not mutated (yet)
console.log({...selectedObject, value: 0.9}.value, 0.9); // Note: This mutates selectedObject on node@11 but not on node@10
console.log(selectedObject.value, 0.25); // Note: selected objects is now mutated!!

On node@11 this outputs this unexpected result:

0.25 0.25
0.9 0.9
0.9 0.25

Where on node@10, it outputs the expected result:

0.25 0.25
0.9 0.9
0.25 0.25

according to my understanding of the description on MDN:

• https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#Spread_in_object_literals

[larsthorup]

This appears to me to be related to JSON.parse(), as the code works correctly with this line instead of the first line:

const workspace = {"orgContentScore":{"41":{"id":"41","contentId":"111","competenceId":"40","scoreTypeId":"6","value":0.25}}};

[BridgeAR]

I reduced the test case and reported it to the @nodejs/v8 team:

const weird = JSON.parse('{"a":0,"b":1,"c":2,"d":3,"e":0.1}');
({...weird, e: 666})
console.assert(weird.e === 0.1);

[BridgeAR]

See https://bugs.chromium.org/p/v8/issues/detail?id=8601. I provided some further details of what I found when looking into this in that issue.

[targos]

It is fixed in V8 7.2.502.4 (from https://github.com/nodejs/node/pull/24875)

[caitp]

Relevant commits which fixed this:
https://github.com/v8/v8/commit/bf84766a2cd3e09070adcd6228a3a487c8dc4bbd,
https://github.com/v8/v8/commit/3e010af274088493f3485d7a16dec4e31550e876

[BridgeAR]

@caitp it seems like the change depends on some other commits. Do you think it would be possible to backport your change?

[caitp]

I could, but I’m on holidays — but, if you feel like taking on fixing it yourself, I’m happy to answer questions.

Otherwise, the folks assigned on the v8 bug will likely get to it before I do.

[hashseed]

@GeorgNeis helpfully provided a patch to V8 7.1 and resolved merge conflicts here. Would anybody be willing to apply this patch to the right branch in Node.js?

[BridgeAR]

@hashseed I updated V8 to the mentioned V8 version first and applied the patch afterwards but it still can't compile :/

I missed something when comparing the patch.