-
-
Notifications
You must be signed in to change notification settings - Fork 504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split between lib/ and es6/ does not work well #1053
Comments
This suggested solution has also been mentioned here: |
I have an alternative suggestion - stop shipping es5 with commonjs modules and only ship es6 with esmodules. Cons
Pros
Personally, tools like rollup and parcel make going back and forth from commonjs to esmodules fairly easy. And tree-shaking (if not optimal) is still achievable with those tools against commonjs dependencies. My biggest qualm with separate esm and cjs builds is that it effectively forces me to also manage two builds for any library I build on top of the fp-ts ecosystem. In my opinion, there is little reason to support two builds, it ends up being busy work both in setup and issue maintenance. Anyway, that's my two cents. |
What really bothers me here - it's extremely easy to screw up when using autoimports because IDE suggests two imports: one from Also I've been always dreaming of throwing commonjs in the trash 😄So I kind of agree with @baetheus |
The important thing is not what we ship, but how it is consumed by runtimes and bundlers. I think it's perfectly reasonable to ship commonjs and esm at the same time. However when different tools import "fp-ts", "io-ts", "fp-ts/Either" etc, they should get a consistent set of modules. Eg. nodejs v12 should get commonjs, nodejs v14 should get esm, rollup should get esm, webpack should get esm, … The first step should be to agree on the module API, ie. what module specifiers do we want people use in their code. For example:
Then we can work out how to tell each tool (nodejs, rollup, webpack etc) where to actually find suitable files. This is done through configuration in package.json files. The next version of nodejs will make that somewhat easier (https://nodejs.org/api/esm.html#esm_conditional_exports), but I'm not aware how rollup or webpack want to support this new field in the future. |
I believe PR #1241 solved a lot problems, the only one I have is that IDE (VSCode) still trying to use lib\es6 folders for auto import, but do not propose just fp-ts/Either |
After a few imports, it figures out the correct path for the import in my case. Feels good! |
Closing this as |
🐛 Bug report
Current Behavior
The following file, when processed by a bundler (rollup, webpack etc), will include the fp-ts Either module twice, once the es6 version and once the lib version.
Why? Because the Either module from monocle imports
fp-ts/lib/Either
, even if itself was imported throughmonocle-ts/es6/Either
.Expected behavior
The fp-ts Either module should appear only once in the output.
Reproducible example
Save the above example as
input.js
, and bundle using the following rollup config:The resulting file will be 144KB, and contain a lot of unnecessary code because rollup can't threeshake commonjs modules.
If I edit the monocle-ts package contents to always import from fp-ts/es6/ instead of fp-ts/lib/, then the size drops to 50K
Suggested solution(s)
The only solution I'm aware of is this: Every module that the user is expected to import in their code has to be a folder, and that folder must contain a package.json which provides pointers to the entry points (
main
being the commonjs entry point, andmodule
being the ES6 module entry point).In other words, in any import statement
import … from "SOMEMODULE"
, the "SOMEMODULE" must refer to a folder with its own package.json, ie.node_modules/SOMEMODULE/package.json
. If SOMEMODULE contains path separators, same applies, eg.import … from 'fp-ts/Either'
->node_modules/fp-ts/Either/package.json
.Additional context
The
@material-ui/core
package solves this in the way I described. You can have a look at the contents of their package here: https://unpkg.com/browse/@material-ui/[email protected]/.Every direct subfolder of this package is allowed to be imported. As you drill down to the subfolders, you'll see a package.json en every one (eg. https://unpkg.com/browse/@material-ui/[email protected]/Avatar/package.json).
The text was updated successfully, but these errors were encountered: