React-spring: Distributing ES6 by default is confusing/premature

[slikts]

Libraries are almost universally distributed as transpiled modules with the node module format as a baseline, so the typical setup is to exclude dependencies in node_modules from transpilation. The documentation states that:

The library comes as an es-module compiled for evergreen browsers ...

Meaning that the use case supported by default is serving ES modules to browsers, which is really bleeding edge and requires a complex setup with bundling as fallback, etc. The experimental module support in node also works differently and uses .mjs files, so there is uncertainty what the proper way of using modules will be going forward.

The issues the current approach causes are, for example, either requiring special config to run in Jest and to be bundled for browsers, or importing the .cjs module paths. It would make more sense and reduce the number of issues users are having to make ES modules be the special case and node modules be the default. It should, as a minimum, be better documented, since having to import from a special path gives the impression of being a workaround.

Examples of issues: #494 #555 #599 and others. #555 in particular gives only a partial solution which works just for Jest but would still fail for bundling or running the code.

[jacobrask]

Maybe we could investigate trying out https://github.com/developit/microbundle or https://github.com/pikapkg/pack

[drcmda]

distribution is a big mess tbh, but i think the way we're doing it is actually fairly common. The lib is transpiled using babel-env on ">1%, not dead, not ie 11, not op_mini all". As for esm, webpack handles esm by default, all bundlers do. You don't need to include node_modules for this

Maybe we could investigate trying out https://github.com/developit/microbundle or https://github.com/pikapkg/pack

Sounds great to me. But i think the problem is the same: no one knows what the baseline is. If we set IE11 as the baseline we add a ton of weight and make the lib slower for 99% of the end-users that use modern browsers. I do think really old browsers are the concern of the end-user. This is the same philosophy that create-react-app follows, their config is:">0.2%, not dead, not ie 11, not op_mini all".

The issues the current approach causes are, for example, either requiring special config to run in Jest and to be bundled for browsers, or importing the .cjs module paths.

I don't think it does that, because Jest reads out package.json and picks the commonjs exports automatically. All SSR and testing libs use the main field, not module.

Examples of issues: #494 #555 #599 and others. #555 in particular gives only a partial solution which works just for Jest but would still fail for bundling or running the code.

All these issues are fixed. It was something else that caused it. When renderprops and hooks had to be two exports, only one could be declared in package.json. So if you use renderprops, jest would pick esm and choke. That happens no more because the default export for renderprops is cjs by default and hooks is fine since gatsby/next/jest pick main, which leads to cjs.

[slikts]

Here's a simple example repo where Jest fails when importing:

It's fixed by importing from react-spring/renderprops.cjs. It's really confusing since renderprops.js is a node module, not ESM.

A different problem that is also fixed by appending .cjs to the import path is UglifyJS failing and can be seen by running webpack; this one's due to UglifyJS only supporting ES5.

This is the same philosophy that create-react-app follows ...

The CRA default config targeting evergreen browsers applies to your own app code; CRA isn't used to develop libraries. The norm for libraries is to provide ES5 by default, and this norm is reflected, for example, in webpack's default config excluding node_modules from transpilation.

The package.json entry point fields only apply to the main or default entry point, so they don't apply to a different entry point like renderprops in this case. I've not tried it, but a solution that would work for non-main entry points could be to have ESM in .mjs files, because .mjs takes precedence over .js in webpack by default, albeit I'm not sure about the other bundlers.

[raspo]

I second this. Updating to react-spring v8 on a project using ts-jest breaks unit tests with the same error reported above.
Thank you @slikts for the react-spring/renderprops.cjs workaround btw

[slikts]

For starters, it'd be useful to understand why the issue is happening in Jest even though the renderprops module is not an ES module.

Something I've realized from this is that UglifyJS only supporting ES5 is a feature, at least if more legacy browsers are targeted, since it prevents untranspiled dependencies from passing the build step.

[drcmda]

Don’t have access to a laptop right now to make sure, but to me it seems renderprops is a commonjs module https://unpkg.com/[email protected]/renderprops it does not have import and export statements so how can jest choke on it?

As for ugliyJS, isn’t it dead? I believe terser is the continued project.

[slikts]

UglifyJS is widely used and still maintained, and most people don't need support for minifying ES6.

[TrySound]

.mjs is bad solution. It's broken in webpack. A good solution is using module field for every path like here
https://unpkg.com/[email protected]/addDays/package.json

Terser is a default in webpack and a lot of other tools. UglifyJS is lost my trust when they gave up on uglify-es (this is how terser was born).

According browsers support. es5 is not a goal for most users these days. The only ancient browser (IE) is almost dead. So if you really need to support it transpile your dependencies like some developers does to support es3.

[slikts]

CIU shows support for ES6 at around 90-95%, so it's usually a business requirement to support those 5-10% of legacy browsers, and I'm reasonably certain that most people don't expect to have to transpile dependencies. UglifyJS not supporting ES6 is a feature, not a flaw, in this context, since untranspiled libraries make it fail at the build step instead of in the browser, which is the worst case scenario.

The date-fns solution is interesting, although it's unfortunate that it requires all the files being named index.js.

[TrySound]

@slikts

5-10% is not a good reason to increase bundle size of 90-50%.

Terser has ecma version options
https://github.com/terser-js/terser#minify-options

Date-fns solution does not require files to be named index.js. You may specify both main and module fields.
https://unpkg.com/[email protected]/Transition/package.json

[slikts]

Do you have a source for the bundle size increase?

terser docs say "this setting is not presently enforced" about the ecma setting.

Thanks for the react-transition-group example; it seems like the all-around best approach.

[TrySound]

It's not about terser. Transpiled classes are much bigger. Function expression is bigger than arrow etc.

[slikts]

I would guess that the size difference from downlevel-compiling to ES5 would be mostly offset by compression, since the code is repetitive. The 90-50% figure seems way too large.

The two relevant issues are distributing both node and ES modules and distributing both ES5 and ESNext code. The module part is solved reasonably well by having separate package.json files, even though .mjs would have less boilerplate, but there's no such standard solution for ES5 vs ESNext code, so it must use different import paths. Currently the different import path for renderprops is misleadingly suffixed with .cjs, even though the actual difference is it being ES5.

The default being ESNext is additionally problematic in that it's a trap for users that support legacy browsers like IE11 and don't expect to have to transpile dependencies, because the norm is to distribute ES5. For example, if my build pipeline didn't involve UglifyJS, importing react-spring/renderprops would make the site fail in legacy browsers, which is obviously really bad, worse than having a larger bundle size.

[drcmda]

could we give it a better name? or maybe adding a rollup ".legacy" slot would do it. you could submit a pr if you like, not opposed to something waterproof in legacy situations. just throwing this on everyones shoulders isn't what i'd like to do.

[drcmda]

PS. the file that builds all of these targets: https://github.com/react-spring/react-spring/blob/master/rollup.config.js

[visualcookie]

Running into issues as well. Don't know if they relate to this, but it looks like it. When trying to build on server which uses SSR, I get the following:

$ NODE_ENV=production node static/server.js
/Users/deanhidri/xxx/xxx/node_modules/react-spring/node_modules/@babel/runtime/helpers/esm/extends.js:1
export default function _extends() {
^^^^^^

SyntaxError: Unexpected token export
    at Module._compile (internal/modules/cjs/loader.js:743:23)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:810:10)
    at Module.load (internal/modules/cjs/loader.js:666:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:606:12)
    at Function.Module._load (internal/modules/cjs/loader.js:598:3)
    at Module.require (internal/modules/cjs/loader.js:705:19)
    at require (internal/modules/cjs/helpers.js:14:16)
    at Object.<anonymous> (/Users/deanhidri/xxx/xxx/node_modules/react-spring/renderprops-addons.js:7:32)
    at Module._compile (internal/modules/cjs/loader.js:799:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:810:10)

[ShanonJackson]

Please, this would fix all the issues i'm having in typescript land with spring in terms of transpiling targetting commonjs

[minheq]

In version 9.0.0-beta.12, when using both Typescript and a react-spring submodule e.g. react-spring/native, the following issues occur when running Jest

1. Jest cannot read out from package.json to use cjs for these submodules.

    /Users/xxx/node_modules/react-spring/native.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){export * from '@react-spring/native';
                                                                                             ^^^^^^

    SyntaxError: Unexpected token export

importing from react-spring works as it is able to interpret from package.json to use cjs

1. Use of the workaround react-spring/native.cjs fails because there is no corresponding typescript definition file

[aleclarson]

@minheq The latest beta fixes all that. Thanks for the report! 👍

[yordis]

@aleclarson would you mind pointing to the PR that fixes the issue?

[azizhk]

Hi @aleclarson can you share what approach react-spring went ahead with in v9?

While using v8, I've changed my imports from "react-spring/renderprops" to "react-spring/renderprops.cjs"

I think the best solution would have been what @TrySound suggested, creating individual package.json for all possible export files. I think that would also work with https://github.com/prateekbh/babel-esm-plugin/ or https://philipwalton.com/articles/using-native-javascript-modules-in-production-today/ without having to transpile anything in node_modules.

[aleclarson]

@azizhk The current v9 beta exports the renderprops API from react-spring (no /renderprops required), and react-spring/web is identical to react-spring, so you should be able to do:

import { Spring } from 'react-spring/web.cjs'

Not sure what the solution for v8 is. I'm not contributing to it.