Feat/exports-maps for consistent behaviour of esm imports in node and browser environments

[JayaKrishnaNamburu]

Thanks to contributors for the awesome project 🚀 .
I have been using it in one of my projects, from a while. And recently i started moving it into pure esm package. And my package targets both browser and node environments.

And so, i noticed how different bundlers and node resolve a package. Here is the reason, node relies on exports map or .mjs extension of a package to consider it as a esm module. But for bundlers it is not needed, since bundlers use module field to do the same. And i see, jss already uses module field 😄

And coming back to my use-case, so here is the tricky part. In Node, it starts to resolve main field and so with the cjs and esm interop in Node js. I need some notation like this to use the packages.

Works good in node, since the only way to import from cjs in a module env in node. Is to import everything and then use jss.default()

import { create } from 'jss'
import preset from 'jss-preset-default'
const jss = create(preset.default())

But, when we bundle the code it breaks. Since, bundlers use module field and resolve the esm.js bundle and so only the below works.

import { create } from 'jss'
import preset from 'jss-preset-default'
const jss = create(preset())

So, we are left with two different ways to import when targeting both environments. Which is not so ideal when same package need to be bundled in both ways.

Solution - 1

To force bundlers to use cjs.js file so both browsers and node env uses the same resolution strategy. Which looks like

resolve: {
    alias: {
      'jss-preset-default': 'jss-preset-default/dist/jss-preset-default.cjs.js'
    }
  }

This helps, but every user with the above use-case like mine need to go through this process.

Solution 2

Adding exports map which helps in resolving dependencies depending on the environment. Fully supported by node and bundlers and esm based CDN's like skypack, jspm

So, i went ahead and made those changes and so now resolving the package will be same in node and browsers.

Summary of changes made

• The esm bundle from rollup should have .mjs extension or the package itself should have type: module property. But since, jss supports umd, cjs too. It's good to have .mjs as extension than switching the whole package to module.
• Updated module field and added exports fields in all packages.
• Upgrade to @babel/[email protected]. Because, below 7.12.0 again it doesn't use exports fields. And again bundlers or node breaks with esm errors. And from 7.12.0 they adopted exports.
• And then upgraded is-in-browser. Again the same, if node tried to resolve jss it imports cjs version of is-in-browser and in latest version 2.0 of is-in-browser we have exports adopted.
• Upgrade webpack and karma-webpack to 5 to support `exports resolution in webpack.

Disadvantages

I personally didn't come across any, since even lower versions of bundlers and node can still work in the same way as they worked before.

PS

Please feel free to close the PR if it's not in the scope, i can solve it using solution-1. But did the changes and opened a PR, so package can adopt and offer by default for future versions 😄

The diff is little big, but most of the changes comes from yarn.lock since i upgraded few packages. Please delete it and generate a new one from your requirements. At the moment 2 tests are breaking, and will look into them if the project can choose to adopt exports.

Thanks again for all the work 😄

[kof]

I am not as deeply involved into the modules mess in our ecosystem, but I think @TrySound is, what do you think?

[kof]

Would dropping cjs actually simplify the situation?

[JayaKrishnaNamburu]

Yes dropping cjs can be one of the solution, but that means the packages need to switch to having type:module field in it. And the main fields needs to point to esm. I will give it a thought today, on what could be implications of totally dropping it. But using exports to provide both makes it seemless for existing users.

[kof]

I would be interested in 2 things:

1. how big is the users group who depend on cjs and why do they still use cjs?
2. do they have an easy fix if we drop it?
3. would we do ecosystem a favour by dropping it? I assume we would
4. would it simplify everything for us? I assume it would

[vovacodes]

I think this guide provides some extensive information on what options are available: https://webpack.js.org/guides/package-exports/

adding “exports” field definitely contributes to the ecosystem of esm and allows tools like jspm and skypack to do their job well and create optimized builds for every entrypoint listed. exports.module is quite important in avoiding the double-inclusion problem https://nodejs.org/api/packages.html#packages_dual_package_hazard .

overall trying to support all existing tools via “exports” is hard and tedious but is very beneficial to the future of ESM adoption

[JayaKrishnaNamburu]

That clarifies a lot of things, so @vovacodes how save is it to drop cjs support to packages at the moment. And going pure esm. I am trying to find the same for my stuff too, so curious to know more about this one.
I mean will the existing users have any issues adopting to it, or since tools supporting now. One can safely drop that.

[JayaKrishnaNamburu]

I would be interested in 2 things:

1. how big is the users group who depend on cjs and why do they still use cjs?
2. do they have an easy fix if we drop it?
3. would we do ecosystem a favour by dropping it? I assume we would
4. would it simplify everything for us? I assume it would

My answer for all the bottom three questions 2,3,4 will be yes. It makes things easier to maintain, yes it's a easy fix for users if they use webpack or rollup or something of that sort which added esm support and can rely on them.

But the only down side is, jest. It is yet to catch-up fully. And the support at the moment is experimental and behind some flags. So, it will create a blocker for users using jest for testing. So, till the other tools catchup. The best solution is providing both as i did in the PR initially.

[kof]

One more thing is missing here, we currently still use webpack for karma tests, which is now failing, try yarn test

[JayaKrishnaNamburu]

One more thing is missing here, we currently still use webpack for karma tests, which is now failing, try yarn test

Sure will fix it, just wanted to know if the library is interested in making the adoption for esm before fixing all the tests failures :)

[kof]

Since it ticks most of the boxes, yes I think we do, but would love to hear what @TrySound thinks

[TrySound]

yeah, better fix failures to not miss some esm issues.

[JayaKrishnaNamburu]

Will be on it 👍

[JayaKrishnaNamburu]

Hi @oleg008 can you helo me in in understanding this test-case alone
https://github.com/cssinjs/jss/blob/master/packages/jss-plugin-isolate/src/index.test.js#L25

I have a very limited knowledge on the internals, and trying to understand the use-case. But still lacking to narrow down. If you can help me with the use-case of jss-plugin-isolate it helps me a lot 😄

[kof]

You mean this particular line? expect(sheets.registry.length).to.be(1) it means 1 style sheet has been registered and attached

should have no reset sheets in registry with reset sheet there would be 2, we generate a reset style sheet in that plugin and this test makes sure none has been generated yet

[JayaKrishnaNamburu]

Thank you, i will try to solve the last test case too 🤞 😄

[JayaKrishnaNamburu]

Heya, i spend the last weekend too on this. Looks like the test fails only when using the esm version of the package. For some reason the sheets inside the StyleSheetRegistry are not resetting. And they keep adding up. For example here
https://github.com/cssinjs/jss/blob/master/packages/jss-plugin-isolate/src/index.test.js#L25

when i import jss from jss/dis/jss.cjs and run the tests. The case passes. But the esm version gives a result of 333 and adds a sheet for every test run :x.
Sorry for the delay, little caught up. Any ideas around this thing will be helpful :)

[kof]

Ok I figured out what the problem here is, it has nothing to do with your change. Tests were not cleaned up everywhere and your change has probably changed the order of execution of tests.

[JayaKrishnaNamburu]

Ok I figured out what the problem here is, it has nothing to do with your change. Tests were not cleaned up everywhere and your change has probably changed the order of execution of tests.

Ahh, any suggestions to get them passing, i have very limited to no knowledge over mocha and stuff. Anything helps :)

[kof]

After trying to fix it, turns out both are a problem - not everything was cleaned up and your change made the dev mode load /tests/utils.js import a different jss, because it uses a relative path.

Most likely before that relative and absolute path was resolving to the same jss build and now they resolve to different once

[kof]

Just discovered this change, why was this necessary with regards to esm imports?

[kof]

Closing this one, here is a new PR based on this #1552

[kof]

I wonder if this can be released with minor version increment or needs a major