Fixes #1968 handling of default export reassignment cases#1975
Fixes #1968 handling of default export reassignment cases#1975lukastaegert merged 1 commit intomasterfrom
Conversation
| } | ||
|
|
||
| assert.deepEqual(actualFiles, expectedFiles); | ||
| assert.deepEqual(Object.keys(actualFiles), Object.keys(expectedFiles)); |
There was a problem hiding this comment.
To my knowledge, there is no guaranteed order for Object.keys. Together with the check below, wouldn't it be enough to check that both have the same length?
There was a problem hiding this comment.
I believe Object.keys is indeed spec'd to enumerate keys in the order they were added. I believe this was an ES6 addition.
There was a problem hiding this comment.
Yes object order can be relied on fine, and it's a nicer error message to get a file listing diff than a numeric diff error.
There was a problem hiding this comment.
In that case, I stand corrected. Sounds like fixturify also has a defined order so no problem here i guess.
There was a problem hiding this comment.
Hi Lukas. So sorry to bother - I know it's Sunday, but is there any chance this will be seeing the light of day soon? I'm eager to continue testing out Rollup's new code splitting 😁
There was a problem hiding this comment.
And sorry for not getting back to you but I was out hiking yesterday 😉I'm on it now!
There was a problem hiding this comment.
Goodness please don't aplogize 🙃
Hope the hike was good - I'll look forward to getting back in there whenever this gets published 😬
3 participants
The
defaultreassignment case requires some exceptions in the code to be handled properly. Ideally we should treat the traced variable of thedefaultas the expression statement of the default itself, as it seems like this inconsistency with the conceptual model is causing these exception cases, but I understand it was used to ensure tracing can get the most sensible variable name in the output which makes sense.The hardest part of cracking this was coming up with just the right chunking scenario to replicate it, but it should be cracked now.
This is quite an important fix as the commonjs plugin outputs this pattern so it would be affecting many users testing out the code splitting with commonjs currently.