sindresorhus/react-extras

Use ESM for build

sholladay opened this issue · 5 comments

sholladay commented

I have been successfully using react-extras v0.7.1 for the past year or so. Today, I tried to upgrade to a more recent version but the build failed. Rollup is having trouble importing components from react-extras v0.9.0 and later due to the convoluted way that Babel is defining the CommonJS exports.

Click here to see the strack trace from Rollup 
[!] Error: 'Choose' is not exported by node_modules/react-extras/dist/index.js
https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module
lib/components/payment/payment-preview.jsx (3:9)
1: import React, { useEffect, useState } from 'react';
2: import PropTypes from 'prop-types';
3: import { Choose } from 'react-extras';
            ^
4: import { Dimmer, Loader, Message } from 'semantic-ui-react';
5: import { load } from '../../constants';
Error: 'Choose' is not exported by node_modules/react-extras/dist/index.js
    at error (/Users/sholladay/Code/<redacted>/client/node_modules/rollup/dist/rollup.js:5363:30)
    at Module.error (/Users/sholladay/Code/<redacted>/client/node_modules/rollup/dist/rollup.js:9701:9)
    at handleMissingExport (/Users/sholladay/Code/<redacted>/client/node_modules/rollup/dist/rollup.js:9618:21)
    at Module.traceVariable (/Users/sholladay/Code/<redacted>/client/node_modules/rollup/dist/rollup.js:10011:17)
    at ModuleScope.findVariable (/Users/sholladay/Code/<redacted>/client/node_modules/rollup/dist/rollup.js:8678:39)
    at ReturnValueScope.findVariable (/Users/sholladay/Code/<redacted>/client/node_modules/rollup/dist/rollup.js:3054:38)
    at ChildScope.findVariable (/Users/sholladay/Code/<redacted>/client/node_modules/rollup/dist/rollup.js:3054:38)
    at Identifier$1.bind (/Users/sholladay/Code/<redacted>/client/node_modules/rollup/dist/rollup.js:4392:40)
    at CallExpression$1.bind (/Users/sholladay/Code/<redacted>/client/node_modules/rollup/dist/rollup.js:3137:31)
    at CallExpression$1.bind (/Users/sholladay/Code/<redacted>/client/node_modules/rollup/dist/rollup.js:6643:15)

Notably, with v0.9.0, node_modules/react-extras/dist/index.js contains:

Object.defineProperty(exports, "Choose", {
  enumerable: true,
  get: function get() {
    return _choose.default;
  }
});

Whereas, with previous versions, it was simply:

exports.Choose = Choose;

This change was introduced in 48b3170.

Needless to say, the former is much harder to statically analyze than the latter. We could try to blame Rollup for not being sophisticated enough, but it doesn't surprise me that it is having trouble with this.

I think the best solution would be for react-extras to publish a build in ES modules format. That way it's easier for modern bundlers to import.

sindresorhus commented

Yes, we can just switch completely to native ES module output.

sholladay commented

Is there a way to do this with just Babel? I didn't quickly find a built-in option that tells it to leave the module format alone.

Could use Rollup, though, I suppose.

sindresorhus commented

You don't need Babel or Rollup. I'm saying this package can be converted to a pure native ESM module.

sholladay commented

I see. I assumed that we would still want to transpile the JSX syntax. If we remove Babel, we lose that, and won't that make it hard for people to use since running Babel on node_modules is uncommon?

I suppose we could just not use JSX. I only see a little bit of it in this package anyway.

sindresorhus commented

The only JSX is here:

react-extras/source/image.js

Lines 5 to 18 in 29fa9e8

<img
{...props}
src={url}
onError={event => {
const element = event.currentTarget;
if (fallbackUrl) {
element.src = fallbackUrl;
} else {
element.style.visibility = 'hidden';
}
}}
/>
);
We could easily change that to plain JS.

  • 👍1