Skip to content
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

Closed
wereHamster opened this issue Dec 17, 2019 · 7 comments
Closed

Split between lib/ and es6/ does not work well #1053

wereHamster opened this issue Dec 17, 2019 · 7 comments

Comments

@wereHamster
Copy link

wereHamster commented Dec 17, 2019

🐛 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.

import { right } from "fp-ts/es6/Either";
import { _right } from "monocle-ts/es6/Either";

console.log(right(42), _right(right(42)));

Why? Because the Either module from monocle imports fp-ts/lib/Either, even if itself was imported through monocle-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:

import commonjs from "rollup-plugin-commonjs";
import nodeResolve from "rollup-plugin-node-resolve";

export default {
  input: "input.js",
  output: { file: "output.js", format: "cjs" },
  plugins: [nodeResolve({}), commonjs({})]
};

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, and module 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/core@4.8.0/.

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/core@4.8.0/Avatar/package.json).

@wmaurer
Copy link
Contributor

wmaurer commented Dec 18, 2019

This suggested solution has also been mentioned here:
gcanti/io-ts#349 (comment)

@baetheus
Copy link
Contributor

baetheus commented Mar 4, 2020

I have an alternative suggestion - stop shipping es5 with commonjs modules and only ship es6 with esmodules.

Cons

Pros

  • A more manageable release target.
  • Reduce issues related to conflicting dependencies.
  • Tree shakeable by default.
  • Smaller package size.
  • End users in commonjs land can still import esm builds.

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.

@raveclassic
Copy link
Collaborator

What really bothers me here - it's extremely easy to screw up when using autoimports because IDE suggests two imports: one from /lib and one from /es6.

Also I've been always dreaming of throwing commonjs in the trash 😄So I kind of agree with @baetheus

@wereHamster
Copy link
Author

[…] stop shipping es5 with commonjs modules and only ship es6 with esmodules.

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:

import … from "fp-ts" // supported
import … from "fp-ts/Either" // supported

import … from "fp-ts/lib/Either" // works, but deprecated
import … from "fp-ts/es6/Either" // works, but deprecated

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.

@Lonli-Lokli
Copy link

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

@waynevanson
Copy link
Contributor

After a few imports, it figures out the correct path for the import in my case.

Feels good!

@gcanti
Copy link
Owner

gcanti commented Mar 12, 2021

Closing this as fp-ts, io-ts and monocle-ts now support the suggested solution.

@gcanti gcanti closed this as completed Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants