Repository navigation
Use add() to append items to activeExports set in HarmonyExportImporedSpecifierDependency - #6950
Conversation
…edSpecifierDependency
|
What about the |
| content += | ||
| "if(" + | ||
| JSON.stringify(activeExports.concat("default")) + | ||
| JSON.stringify(activeExports.add("default")) + |
There was a problem hiding this comment.
activeExports must be serialized into an array:
Array.from(activeExports).concat("default")
| content += | ||
| "if(" + | ||
| JSON.stringify(activeExports.concat("default")) + | ||
| JSON.stringify(Array.from(activeExports.add("default"))) + |
There was a problem hiding this comment.
Technically this is correct, but using the return value of add is kind of weird to me.
Better use JSON.stringify(Array.from(activeExports).concat("default"))
ooflorent
left a comment
ooflorent
left a comment
There was a problem hiding this comment.
Please use concat instead of add
|
@mohsen1 Thanks for your update. I labeled the Pull Request so reviewers will review it again. @ooflorent Please review the new changes. |
| return "dynamic-reexport"; | ||
| } | ||
| }; | ||
| should(instance.getContent(dep)).to.eql("???"); |
There was a problem hiding this comment.
Where can I learn what this module does? I'm absolutely clueless how this code is structured
|
Only integration test contribute to coverage. Unittests are just for fun. Code like this could trigger that change: export var x = 1;
export * from ",/cjs";// cjs.js
module.exports = { x: 2, y: 3 }; |
|
|
||
| import {x, y} from "./a"; | ||
|
|
||
| it("namespace export as from commonjs should override named export", function() { |
There was a problem hiding this comment.
No, the spec says * export should have lower precedence than direct export. Order of exports doesn't matter here.
So the correct result would be x = 1
|
|
||
| import {x, y} from "./a"; | ||
|
|
||
| it("namespace export as from commonjs should override named export", function() { |
There was a problem hiding this comment.
No, the spec says * export should have lower precedence than direct export. Order of exports doesn't matter here.
So the correct result would be x = 1
|
I fixed it for you |
|
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
|
Thanks |
4 participants
What kind of change does this PR introduce?
Bug fix
Did you add tests for your changes?
no
If relevant, link to documentation update:
n/a
Summary
This is part of #6862
TypeScript compiler found that we're calling
concaton aSethereDoes this PR introduce a breaking change?
No
Other information