Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESM + Jest #20

Closed
milesj opened this issue Mar 26, 2021 · 10 comments
Closed

ESM + Jest #20

milesj opened this issue Mar 26, 2021 · 10 comments

Comments

@milesj
Copy link

milesj commented Mar 26, 2021

Because this package is ESM only now and doesn't pre-compile, it complete breaks all unit testing since most (if not all) of them are unable to import ESM files.

    Jest encountered an unexpected token

    This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.

    By default, if Jest sees a Babel config, it will use that to transform your files, ignoring "node_modules".

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/en/ecmascript-modules for how to enable it.
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/en/configuration.html

    Details:

    /Users/milesj/Projects/aesthetic-react/node_modules/direction/index.js:16
    export function direction(value) {
    ^^^^^^

    SyntaxError: Unexpected token 'export'

      24 |
      25 | var React__default = /*#__PURE__*/_interopDefaultLegacy(React);
    > 26 |
         | ^
      27 | var getDirection__default = /*#__PURE__*/_interopDefaultLegacy(getDirection);
      28 |
      29 | var hoistNonReactStatics__default = /*#__PURE__*/_interopDefaultLegacy(hoistNonReactStatics);

      at Runtime.createScriptFromCode (../../node_modules/jest-runtime/build/index.js:1350:14)
      at Object.<anonymous> (../core-react/lib/index.js:26:20)
@wooorm
Copy link
Owner

wooorm commented Mar 26, 2021

ref: wooorm/markdown-table#22.

I’ve slowly started pushing ESM from the low-level tools outwards.

The readme + these Jest links show some solutions.

Jest is having a lot of troubles with it though. Here’s one solution with TS and ESM: https://github.com/kentcdodds/mdx-bundler/blob/ee6c8d9981df93aa519f662617278e1ec3f9b2da/jest.config.js

@wooorm wooorm changed the title v2 ESM doesnt work with unit testing ESM + Jest Mar 26, 2021
@wooorm
Copy link
Owner

wooorm commented Mar 28, 2021

Closing as it works as expected, there’s ways around it, and you don’t have to change just yet if you don’t feel like it!

@wooorm wooorm closed this as completed Mar 28, 2021
@milesj
Copy link
Author

milesj commented Mar 29, 2021

I disagree that it's working as intended. I'll just stick with v1 indefinitely.

@wooorm
Copy link
Owner

wooorm commented Mar 29, 2021

I'll just stick with v1 indefinitely.

What’s your reasoning for never wanting to use ESM?

@milesj
Copy link
Author

milesj commented Mar 29, 2021

This is a library that can be used in both web and node contexts, and should work in both of them without issue. As of right now, this is not the case. I'd also say that ESM only isn't offering any benefit over the v1 code.

IMO, it's also not a good pattern to expect all downstream consumers to reconfigure their tooling for a single library to be able to run.

@wooorm
Copy link
Owner

wooorm commented Mar 29, 2021

This is a library that can be used in both web and node contexts, and should work in both of them without issue.

ESM can be used in both. CJS can’t. This is specifically true for this project. For projects w/ dependencies, import maps (native, proposal) or a build step would still be needed. However, that’s already the case for CJS. ESM brings us closer to that interop you’re talking about.

As of right now, this is not the case

Incorrect. This project can be loaded in browsers. It couldn’t before.

I'd also say that ESM only isn't offering any benefit over the v1 code.

Not much, no, for this project. But the ecosystem moving towards a single import/export syntax is a good thing. I’m pretty happy with CJS, and would’ve been fine with not changing anything and instead browsers adopting it. But, ESM happened and it is better. It is the path forward.

IMO, it's also not a good pattern to expect all downstream consumers to reconfigure their tooling for a single library to be able to run.

I’m not suggesting to change for a single project.
The shift is happening. Folks are changing to ESM. Even if that didn’t happen for existing tools, I think it’s allowed for developers of new projects to choose ESM. Users of dependencies will have to deal with it some time.
And existing tools will change. Soon. Did you see https://twitter.com/sindresorhus/status/1349294527350149121?

@milesj
Copy link
Author

milesj commented Mar 29, 2021

@wooorm Yeah I'm well aware of the current state of modules in the frontend ecosystem. But that's also the problem, we're not 100% on ESM yet, so supporting both/all variants is good practice until we hit that threshold. Just my 2 cents.

I think once all popular tooling supports ESM natively, this will be a non-issue.

@wooorm
Copy link
Owner

wooorm commented Mar 30, 2021

I think once all popular tooling supports ESM natively, this will be a non-issue.

What’s the threshold here? When is it OK?

A lot of the tooling does already support it. Babel, Rollup, Vite, esbuild, tape, Node, ESLint, TS. I’ve had a great experience switching. April 30 is an important date, when Node 10 is EOL. Electron and Next are gearing up for that date too.

I’m seeing a couple of projects struggling: Jest and Webpack. IMO it’s really on them, they’ve supported faux-ESM for years and never bothering to make actual ESM work. It’s called ES2015 modules. These projects have had the time. I think a nudge might help push these projects to support ESM.

But that's also the problem, we're not 100% on ESM yet, so supporting both/all variants is good practice until we hit that threshold

I’ve had a terrible experience with dual publishing. Webpack 4 doesn’t handle it well. And there’s https://nodejs.org/api/packages.html#packages_dual_commonjs_es_module_packages.

All these projects (that I maintain, such as direction) used to have browser builds, Bower and Component.js manifests... It’s a lot of work.

For “until we hit that threshold”, agreed, but my perspective is that we reach that in 31 days

@milesj
Copy link
Author

milesj commented Mar 30, 2021

What’s the threshold here? When is it OK?

When they all support it. 🤷

This is all wishful thinking but not realistic. Don't mean to cause conflict, just the current state of things is non-ideal.

I'll just wait till everything catches up.

@wooorm
Copy link
Owner

wooorm commented Mar 30, 2021

just the current state of things is non-ideal.

Yep!

I'll just wait till everything catches up.

I think that’s totally valid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants