Distributing ES6 by default is confusing/premature #601

slikts · 2019-03-19T11:52:31Z

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.

The text was updated successfully, but these errors were encountered:

jacobrask · 2019-03-19T12:57:14Z

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

drcmda · 2019-03-19T14:55:39Z

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 · 2019-03-21T06:31:43Z

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 · 2019-03-21T19:18:15Z

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 · 2019-03-22T06:57:38Z

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 · 2019-03-23T08:18:18Z

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 · 2019-03-23T13:20:36Z

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

TrySound · 2019-03-23T13:44:00Z

.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 · 2019-03-23T14:15:54Z

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 · 2019-03-23T14:20:09Z

@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 · 2019-03-23T14:26:40Z

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 · 2019-03-24T17:00:45Z

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

slikts · 2019-03-25T06:32:56Z

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 · 2019-03-25T14:43:16Z

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 · 2019-03-25T14:44:04Z

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

React Spring distributes ES6 by default, which breaks jest tests unless it's imported explicitly using the cjs extension. See pmndrs/react-spring#601

visualcookie · 2019-04-03T13:19:16Z

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 · 2019-04-11T20:58:59Z

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

minheq · 2019-06-12T04:12:48Z

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

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

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

aleclarson · 2019-06-12T14:08:42Z

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

yordis · 2019-11-03T07:35:54Z

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

azizhk · 2019-12-02T11:51:09Z

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 · 2019-12-02T18:16:25Z

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

m98 · 2021-04-03T12:34:47Z

@aleclarson in this comment you mentioned:

The current v9 beta exports the renderprops API from react-spring (no /renderprops required)

But in the documentation, it still uses renderprops to import Transition

Is documentation outdated? or we should use react-spring/renderprops?

joshuaellis · 2021-04-03T13:07:43Z

I would check the link in the repo for more up to date documentation. Updating the site is on my list of to-dos.

Luchanso · 2021-04-03T15:56:18Z

The same problem in v9.0.0, CI logs with jest. workaroud solution is '.cjs' in the end of import, but you lose Types for typescript 😞

joshuaellis · 2021-04-03T16:16:29Z

@Luchanso if you want to open a new issue and follow the guide, we can look into it when we get to it 😄

m98 · 2021-04-03T16:17:26Z

@joshuaellis Is the website made from .md files on this repository? (I just could not find the website on this repo)

Luchanso · 2021-04-03T16:28:23Z

@Luchanso if you want to open a new issue and follow the guide, we can look into it when we get to it 😄

Thank you 😃 I found solution of my problem

joshuaellis · 2021-04-03T17:05:08Z

@joshuaellis Is the website made from .md files on this repository? (I just could not find the website on this repo)

No, there's the current (outdated) website react-spring.io, then there is a small microsite containing some documentation on v9 – https://aleclarson.github.io/react-spring/v9/, unfortunately, this was the way it was left....

slikts mentioned this issue Mar 21, 2019

Importing a node module from react-spring/renderprops transforms it into an ES module and fails to run in node jestjs/jest#8186

Closed

slikts changed the title ~~Distributing ES modules by default is confusing/premature~~ Distributing ES6 by default is confusing/premature Mar 23, 2019

aleclarson added the discussion label Apr 1, 2019

aleclarson mentioned this issue Apr 1, 2019

Bug since v8 with @babel/runtime and cjs module #599

Closed

mrfelton added a commit to LN-Zap/zap-desktop that referenced this issue Apr 1, 2019

fix(deps): import react-spring using cjs

c59f492

React Spring distributes ES6 by default, which breaks jest tests unless it's imported explicitly using the cjs extension. See pmndrs/react-spring#601

aleclarson closed this as completed Jun 12, 2019

wbobeirne mentioned this issue Feb 26, 2020

v9.0.0-beta.34 • Feedback wanted #642

Closed

aleclarson mentioned this issue Mar 18, 2020

I get the following error on build only... TypeError: b is not a function #947

Closed

aleclarson mentioned this issue Apr 18, 2020

NextJS compile error #961

Closed

aleclarson mentioned this issue May 13, 2020

v9.0.0 #632

Closed

5 tasks

s7dhansh mentioned this issue May 18, 2020

Avoid bailing out with module.useNode() for ESM modules. meteor/meteor#10926

Merged

aleclarson mentioned this issue Jun 19, 2020

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module #1062

Closed

imyjimmy mentioned this issue Jul 24, 2020

Error: Element type is invalid when using Parallax v9.0.0-rc.3 #1090

Closed

diegopinate mentioned this issue Aug 25, 2020

React-Spring CommonJS import for Jest iTwin/viewer-components-react#45

Merged

stopyransky mentioned this issue Oct 8, 2021

SCSS error when importing a react shared-ui lib component into Next.js app nrwl/nx#7203

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Distributing ES6 by default is confusing/premature #601

Distributing ES6 by default is confusing/premature #601

slikts commented Mar 19, 2019 •

edited

Loading

jacobrask commented Mar 19, 2019

drcmda commented Mar 19, 2019 •

edited

Loading

slikts commented Mar 21, 2019

raspo commented Mar 21, 2019

slikts commented Mar 22, 2019

drcmda commented Mar 23, 2019 •

edited

Loading

slikts commented Mar 23, 2019

TrySound commented Mar 23, 2019

slikts commented Mar 23, 2019

TrySound commented Mar 23, 2019

slikts commented Mar 23, 2019

TrySound commented Mar 24, 2019 •

edited

Loading

slikts commented Mar 25, 2019 •

edited

Loading

drcmda commented Mar 25, 2019

drcmda commented Mar 25, 2019

visualcookie commented Apr 3, 2019

ShanonJackson commented Apr 11, 2019

minheq commented Jun 12, 2019

aleclarson commented Jun 12, 2019

yordis commented Nov 3, 2019

azizhk commented Dec 2, 2019

aleclarson commented Dec 2, 2019

m98 commented Apr 3, 2021

joshuaellis commented Apr 3, 2021

Luchanso commented Apr 3, 2021

joshuaellis commented Apr 3, 2021

m98 commented Apr 3, 2021

Luchanso commented Apr 3, 2021

joshuaellis commented Apr 3, 2021

Distributing ES6 by default is confusing/premature #601

Distributing ES6 by default is confusing/premature #601

Comments

slikts commented Mar 19, 2019 • edited Loading

jacobrask commented Mar 19, 2019

drcmda commented Mar 19, 2019 • edited Loading

slikts commented Mar 21, 2019

raspo commented Mar 21, 2019

slikts commented Mar 22, 2019

drcmda commented Mar 23, 2019 • edited Loading

slikts commented Mar 23, 2019

TrySound commented Mar 23, 2019

slikts commented Mar 23, 2019

TrySound commented Mar 23, 2019

slikts commented Mar 23, 2019

TrySound commented Mar 24, 2019 • edited Loading

slikts commented Mar 25, 2019 • edited Loading

drcmda commented Mar 25, 2019

drcmda commented Mar 25, 2019

visualcookie commented Apr 3, 2019

ShanonJackson commented Apr 11, 2019

minheq commented Jun 12, 2019

aleclarson commented Jun 12, 2019

yordis commented Nov 3, 2019

azizhk commented Dec 2, 2019

aleclarson commented Dec 2, 2019

m98 commented Apr 3, 2021

joshuaellis commented Apr 3, 2021

Luchanso commented Apr 3, 2021

joshuaellis commented Apr 3, 2021

m98 commented Apr 3, 2021

Luchanso commented Apr 3, 2021

joshuaellis commented Apr 3, 2021

slikts commented Mar 19, 2019 •

edited

Loading

drcmda commented Mar 19, 2019 •

edited

Loading

drcmda commented Mar 23, 2019 •

edited

Loading

TrySound commented Mar 24, 2019 •

edited

Loading

slikts commented Mar 25, 2019 •

edited

Loading