Fix exports walk (#852) #853
Conversation
🦋 Changeset detectedLatest commit: 9a9b45c 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 |
There was a problem hiding this comment.
This does start to make me worry that #784 was a bad idea, due to the amount of possible combinations that we can't reasonably cover, requiring users to fix filenames on their end.
src/index.js
Outdated
| @@ -260,6 +260,7 @@ function replaceName(filename, name) { | |||
| } | |||
|
|
|||
| function walk(exports) { | |||
| if (!exports) return null; | |||
| if (typeof exports === 'string') return exports; | |||
| return walk(exports['.'] || exports.import || exports.module); | |||
There was a problem hiding this comment.
Hmm... I found a regression when declaring modern exports using default conditional export, such as:
"exports": {
".": {
"import": "dist/index.js",
"require": "dist/index.cjs",
"default": "dist/index.modern.js"
}
}The modern module is not being generated in version >= 0.13.2 while it is being generated in v0.13.1. Probably, we can also add exports.default inside the walk recursion method. wdyt @developit ? 🙇
| return walk(exports['.'] || exports.import || exports.module); | |
| return walk(exports['.'] || exports.import || exports.module || exports.default); |
There was a problem hiding this comment.
Hi @jeffryang24 - no update, just haven't gotten around to it yet.
There was a problem hiding this comment.
I think we could add .default as you suggest, but really this needs to also be checking the extension based on pkg.type, otherwise it will potentially return a CommonJS entry filename for the modern filename.
I have a fairly large overhaul of this logic that uses a complete Package Exports implementation to determine names, but I haven't had a chance to get tests passing for it yet.
There was a problem hiding this comment.
I see... got it... Currently, I've reverted back to 0.13.1 to fix this regression issue. Thanks for your explanation... 🙇
There was a problem hiding this comment.
@jeffryang24 thanks, I also reverted to 0.13.1 and that makes it work for me for the moment.
Fixes #852