use correct fields for entry points in package.json

[TheLudd]

The fantasy-land keys are currently not importable when using esmodules available in node 12 and forward, these changes solve the problem.

The module field has been removed since it serves no purpose and is not recognized by node. Exports field has been added accoding to the official node js documentation: https://nodejs.org/api/packages.html#packages_conditional_exports

Main field is kept for legacy suppoert of node versions 12 and earlier.

The types field has been removed since it is not needed. Typescript will find the definitions anyway when using both import and require.

This should in theory not be breaking but it is difficult to know if any use case or build tool relies on the undocumented conventions that previoulsy existed in package.json.

[davidchambers]

Thank you, @TheLudd. :)

I would be grateful if you would have a look at this, @Avaq, as you have a lot of relevant experience. :)

[Avaq]

The module field has been removed since it serves no purpose and is not recognized by node.

The module field was used mainly by bundlers like Webpack, Rollup, etc. In my experience, these tools are slow to update, and their users even slower to install the updated versions. For that reason I chose the seemingly safer approach of just leaving the field in the manifests of the packages that I maintain, although I did no actual research to determine the real impact that makes.

Are you sure it's safe to remove this field, @TheLudd ?

The rest looks good to me.

[Avaq]

This should in theory not be breaking

Wouldn't it break for users who are currently importing the cjs (auto compatibility in Node) version using import fl from 'fantasy-land'?

[TheLudd]

Wouldn't it break for users who are currently importing the cjs (auto compatibility in Node) version using import fl from 'fantasy-land'?

That is right, that will not work, but does it today? The correct way to import should be import * as fl from 'fantasy-land'

[TheLudd]

The module field was used mainly by bundlers like Webpack, Rollup, etc. In my experience, these tools are slow to update, and their users even slower to install the updated versions. For that reason I chose the seemingly safer approach of just leaving the field in the manifests of the packages that I maintain, although I did no actual research to determine the real impact that makes.

I tried with rollup and that works without the module field. I have no experience of webpack so I could not test it to be sure. Someone familiar with webpack can perhaps pull my branch and use a local linked version of fantasy-land to test budling.

We can of course leave it to be on the safe side. But it is explicitly stated in the nodejs documentation that the module field is not supported. https://nodejs.org/api/packages.html#packages_dual_commonjs_es_module_packages

...Node.js ignored (and still ignores) the top-level "module" field.

We can also release a new major version that "should not really cause any issues".

[Avaq]

That is right, that will not work, but does it today?

I believe it does, thanks to Node's CJS interrop

To make it non-breaking for these users, a default export like export default {map, chain, ...} would need to be added to the esm file.

[davidchambers]

To make it non-breaking for these users, a default export like export default {map, chain, ...} would need to be added to the esm file.

We could update generate-es like so:

(
  awk '{ print "export const " $0 " = \047fantasy-land/" $0 "\047;" }' names
  echo
  echo 'export default {'
  awk '{ print "  " $0 "," }' names
  echo '}'
) >index.mjs

[TheLudd]

I think that is because today it will go through the main (umd) file.

When using the esm file, you have never been able to do import fl from 'fantasy-land'

If I use rollup with the latest released version of fl and explicitly reuire the esm file I get this error:

My code:

import fl from 'fantasy-land/index.mjs'

console.log(fl)

The error

[!] Error: 'default' is not exported by node_modules/fantasy-land/index.mjs, imported by app.js https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module app.js (1:7) 1: import fl from 'fantasy-land/index.mjs' ^ 2: 3: console.log(fl) Error: 'default' is not exported by node_modules/fantasy-land/index.mjs, imported by app.js

[TheLudd]

I believe it does, thanks to Node's CJS interrop

Ok, so that means that my changes could be considered breaking.

Now, I do not like exporting both default and named, I'd rather go with a new major version. In order to go fully esm you will need to do a few changes in your code anyway, like importing full file paths (endin in .js and such). And the intended way of importing a full object from named exports is by using the * pattern.

So my vote is on a new major version. There is a lot of confusion out there about esm and I like packages who do it the right way.

That said, if you are set on default export, I can fix that too.

[Avaq]

Now, I do not like exporting both default and named, I'd rather go with a new major version.

I agree. Adding the default export is messy overhead for what it's worth. I just wanted to make sure it's clear that this change is going to be breaking for a (probably tiny) subset of users.

[davidchambers]

We seem to be discussing two changes that need not be made together. I suggest moving the breaking change to a separate pull request. We could then merge this pull request and release a new version of the package right away. What do you think?

[Avaq]

We seem to be discussing two changes that need not be made together. I suggest moving the breaking change to a separate pull request. -- @davidchambers

I don't see how. The change proposed in this pull request (specifying the correct entry points in package.json) is the breaking change. We have discussed a separate change that could be made on top of it, in order to preserve backwards compatibility. Both @TheLudd and I are however in agreement that going forward with the breaking change (made in this PR) without including the compatibility patch is the way to go.

[davidchambers]

Thanks, Aldwin. It seems this pull request is ready to merge. :)

Could one of you summarize this pull request in a sentence, for the release notes?

[TheLudd]

Could one of you summarize this pull request in a sentence, for the release notes?

Make fantasy-land compatible with native es modules.

[TheLudd]

Thanks for merging. Is it possible to get a version 5 out?

[davidchambers]

https://github.com/fantasyland/fantasy-land/releases/tag/v5.0.0