Repository navigation
Fix importConditionDefaultExport - #546
Conversation
🦋 Changeset detectedLatest commit: ae70567 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #546 +/- ##
==========================================
+ Coverage 88.70% 91.66% +2.95%
==========================================
Files 37 37
Lines 1664 1691 +27
Branches 465 474 +9
==========================================
+ Hits 1476 1550 +74
+ Misses 180 132 -48
- Partials 8 9 +1
☔ View full report in Codecov by Sentry. |
| // the * won't really do anything right now | ||
| // since cjs-module-lexer won't find anything | ||
| // but that could be fixed by adding fake things | ||
| // to the .cjs.js file that look like exports to cjs-module-lexer | ||
| // but don't actually add the exports at runtime like esbuild does | ||
| // (it would require re-running dev when adding new named exports) |
There was a problem hiding this comment.
I don't understand what's happening here but it sounds like it does nothing today so that's... fine? 😅
There was a problem hiding this comment.
correct (this bit is also not new to this PR, it's just moved around)
The comment is generally about making this work in preconstruct dev:
// somewhere.mjs
import { something } from 'my-pkg'It currently doesn't (and didn't before any of the importConditionDefaultExport stuff).
There was a problem hiding this comment.
If I understand correctly, the core of this PR is to avoid loading CJS file directly into MJS because TS doesn't understand module but yet it understands import and changes what kind of a default we get there (tangent... is this even OK? i guess that it might be because it kinda acts as module here even though not through that condition, OTOH... this isn't how strict bundlers work, I think). To avoid this we create an extra CJS file to normalize the default thing so it can mean the same thing for the runtime and types in more scenarios
|
Yes, your understanding is correct though it's specifically about the |
The previous version didn't work with
"moduleResolution": "bundler", this fixes that.