fix: remove exports["."].module to be compatible with resolution policy of nodejs offical & bundlers like vite
#13
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 220b353:
|
package.json
Outdated
| @@ -19,7 +19,7 @@ | |||
| "./package.json": "./package.json", | |||
| ".": { | |||
| "types": "./dist/src/index.d.ts", | |||
| "module": "./dist/index.modern.js", | |||
| "module": "./dist/index.modern.mjs", | |||
There was a problem hiding this comment.
It was intentional to have .js, otherwise it will break in some old bundlers.
Now, time passed. I think if this causes a problem, we should remove it entirely.
| "module": "./dist/index.modern.mjs", |
Does that fix?
There was a problem hiding this comment.
@dai-shi yes, it does fix.
Or could we add "type": "module" to package.json? this can also fix this issue
There was a problem hiding this comment.
It was intentional to have
.js, otherwise it will break in some old bundlers. Now, time passed. I think if this causes a problem, we should remove it entirely.
does that old bundler consume exports field? what I changed is only the one in exports field, the module field is kept as it is now
There was a problem hiding this comment.
"type": "module" might also break old bundlers. We keep cjs default for all jotai packages.
Now, would you be willing to fix more jotaijs/* packages? Some of them are not actively maintained. If the repo is migrated to pnpm from yarn, it means it's maintained.
There was a problem hiding this comment.
@dai-shi I find another solution, we can just reorder the keys of exports["."], please review latest commit
There was a problem hiding this comment.
The order was important for a bundler to pick "module" before "import".
So, instead of reordering, let's remove it.
Alright, let's wait for other contributors for other repos.
There was a problem hiding this comment.
👌 I've removed it
There was a problem hiding this comment.
Thanks.
Can you also remove "postcompile" script because we no longer need it?
There was a problem hiding this comment.
Thanks. Can you also remove "postcompile" script because we no longer need it?
I don't think so as "module": "./dist/index.modern.js" at Line 16 is not removed
There was a problem hiding this comment.
Oh, nice catch. Yeah, that might be good for very old bundlers (though, I'm not sure if we need to keep supporting it.)
indooorsman
changed the title
exports["."] to be compatible with resolution policy of nodejs offical & bundlers like vite
indooorsman
changed the title
exports["."] to be compatible with resolution policy of nodejs offical & bundlers like viteexports["."].module to be compatible with resolution policy of nodejs offical & bundlers like vite
some build tools like
vitewould fail if type of package is not explicit declared while file extension is.js:@dai-shi please help review, thanks