fix: Fix missing support for named re-exports #53
Conversation
| }); | ||
|
|
||
| expect(output.code).toBe(REEXPORT_OUTPUT); | ||
| }); |
There was a problem hiding this comment.
Added a test case for the missing CJS interop wrapping, now supported.
| minimatch(value, dependency), | ||
| ); | ||
| } | ||
| return matchedDependencies[value]; |
There was a problem hiding this comment.
This is an unrelated optimization; Some memoizing here speeds the plugin to a good amount. This makes our builds 1s faster with no apparent downside.
| const preambles: string[] = []; | ||
| let hasDynamicImportsToFix = false; |
There was a problem hiding this comment.
For simplicity, all fixes are included in toBeFixed.
The dynamicImportsToBeFixed is turned to a hasDynamicImportsToFix boolean to replace the one case where we used it as such (if dynamicImportsToBeFixed.length)
| } | ||
| } | ||
| break; | ||
| default: |
There was a problem hiding this comment.
This adds support for ExportNamedDeclaration and applies the same logic to all nodes to be fixed to avoid duplicating code.
| return; | ||
| } | ||
| const bottomUpToBeFixed = toBeFixed.reverse(); | ||
|
|
||
| const ms = sourcemaps ? new MagicString(code) : null; | ||
| let counter = 1; | ||
| let specifierCounter = 1; |
There was a problem hiding this comment.
We need a unique counter for ExportNamedDeclaration specifiers.
| code.slice(node.end); | ||
| } | ||
| continue; | ||
| } |
There was a problem hiding this comment.
This code was just moved. This is a slight behavior change; We no longer process dynamicImportsToBeFixed before bottomUpToBeFixed. Instead, we process all fixes in the (reversed) order that they are found in the module... Which seemed actually safer to me. But please review carefully.
| continue; | ||
| } | ||
|
|
||
| if (node.type === "ExportNamedDeclaration") { |
There was a problem hiding this comment.
All the code inside that if block is the new feature being added to handle re-exports.
The library is currently missing re-export patterns where it isn't applying CJS interop correctly:
We discovered this upon investigating some build failures. Seemed like an helpful fix to bring back upstream.