ES modules aren't supported

[MarkBennett]

Describe the bug
I was trying to use the newest version of node-fetch but in doing so discovered that Redwood does not support require() of ES Modules right now when Redwood tried to compile and start my api service.

To Reproduce
Steps to reproduce the behavior:

1. Install and import a pure ES module (ESM)
2. Run yarn rw dev
3. Wait a moment while things start up
4. See error error when node tries to start the api side

You can see a discussion with further details and sample source on the Redwood Discourse where I discussed this issue with @dthyresson and shared more detailed code samples.

Expected behavior
I would expect that ES modules will "just work" given they're getting increasingly popular as people write libraries for node, web, and deno.

Screenshots
N/A

Additional context

• Discussion with code samples on Redwood Discourse

[jtoar]

Right now Redwood indeed doesn't support ES Modules. We plan to support them but we haven't thoroughly consider what that would entail. Since Redwood isn't consumed by anything downstream, we may actually have an easier time converting to ESM, but it would be pure ESM I think.

From reading the thread, it doesn't look like you actually tried to require an ES Module. Rather, it looks like your ES6 syntax (import fetch from 'node-fetch') was transpiled into a require statement (maybe something like const fetch = require('node-fetch')). But just note that you can't actually require an ES Module:

Using require to load an ES module is not supported because ES modules have asynchronous execution. Instead, use import() to load an ES module from a CommonJS module.

Source: https://nodejs.org/api/esm.html#require.

If you don't mind, can I update the title of the issue to reflect the problem more generally?

[MarkBennett]

If you don't mind, can I update the title of the issue to reflect the problem more generally?

Please! This is above my limited understanding of the issue, so please do what you need @jtoar. Thanks!

[MarkBennett]

From reading the thread, it doesn't look like you actually tried to require an ES Module. Rather, it looks like your ES6 syntax (import fetch from 'node-fetch') was transpiled into a require statement (maybe something like const fetch = require('node-fetch')). But just note that you can't actually require an ES Module:

Are you aware of a way to prevent this transiplation? Or would you say that's the actual issue in this case @jtoar?

[jtoar]

Are you aware of a way to prevent this transiplation?

Yes, it's actually pretty simple. Babel's preset-env preset has an option, modules, which you can set to false to tell it to preserve ES modules. Source: https://babeljs.io/docs/en/babel-preset-env#modules.

would you say that's the actual issue?

Great question! Probably not. Telling Babel not to transpile is actually the easy part.

Chores aren't the main issue, but there are a lot of them, like adding file extensions to relative imports (ES modules don't like bare specifiers, or something like that). More generally, we'd have to go through the whole codebase and refactor things like __dirname.

We haven't prioritized ES modules because we've tried to avoid touching our Babel/Webpack/Jest/Storybook configs as we fix bugs for v1. I'd say that's the main issue. And I'm sure pretty it's taken Jest a while to support ES modules. They're close but I don't think they're quite there yet.

This is something we'll have to do, and want to do moreover. Does that kind of answer your question? We'd appreciate any investigation into this! But it's kind of a gargantuan task 😅

[MarkBennett]

This is something we'll have to do, and want to do moreover. Does that kind of answer your question? We'd appreciate any investigation into this! But it's kind of a gargantuan task 😅

How would one go about documenting the work required? Could I change that preset then make a list of what breaks?

Is this issue the best place to track or do we need to make it a meta issue if there are dependencies on things like Jest as you say?

It looks like Jest has an open issue tracking this as you say...

jestjs/jest#9430

[jtoar]

Hey @MarkBennett sorry it took me so long to get back to you.

Could I change that preset then make a list of what breaks?

You're certainly welcome to! This is basically how I went about it too. Here's the incomplete list I came up with:

• are we shipping both ESM and CommonJS, or just ESM?
  • Redwood packages aren't consumed by anything downstream so I think we'd have an easy, non-breaking time shipping just ESM. The version of node we target is high enough too.
• refactor or polyfill __filename and __dirname
  • I'd prefer to refactor these, the new way of doing things really isn't that bad: https://nodejs.org/dist/latest-v16.x/docs/api/esm.html#no-__filename-or-__dirname
  • The task at hand here is making sure all our path, I/O logic still works (think: generators, finding config files, Windows)
• there's some lazy requires in the CLI—those have to be dynamic imports now: https://nodejs.org/dist/latest-v16.x/docs/api/esm.html#import-expressions, and anywhere there's a module.exports and/or a require.resolve needs refactoring
• add file extensions to relative and absolute specifiers: https://nodejs.org/dist/latest-v16.x/docs/api/esm.html#mandatory-file-extensions
• Upgrade dependencies we've been ignoring because they're ESM only:

  redwood/.github/renovate.json

  Lines 16 to 28 in 7d2d284

  	"boxen",
  	"configstore",
  	"decamelize",
  	"execa",
  	"humanize-string",
  	"latest-version",
  	"ora",
  	"tempy",
  	"terminal-link",
  	"@types/node-fetch",
  	"chalk",
  	"pascalcase",
  	"node-fetch",

• Figure out what to do about Jest compatibility
  • This is probably where we'll have the most trouble

This is definitely an incomplete list, and maybe everything here could be approached differently using something like rollup. The task at hand like you said is to just make sure everything still works, so there's a ton of unknowns.

I'll try to put up a PR I was working on in the next few days for you to check out if you're interested!

[jonschlinkert]

• refactor or polyfill __filename and __dirname

I've had to do this a lot, figured maybe I could save you 20 seconds:

export const __filename = new URL(import.meta.url).pathname;
export const __dirname = path.dirname(new URL(import.meta.url).pathname);

[ebramanti]

Would be interested in getting this working. A lot of @sindresorhus' packages have now moved to only supporting ESM. I am unable to use https://github.com/sindresorhus/p-map in my project.

[dennemark]

Hi,
I also have an ESM only module that I need to use on api side. This creates issues in yarn rw dev as well as yarn rw test. I can pass custom node arguments via script tag in packages.json i.e. : "test": "NODE_OPTIONS=--experimental-vm-modules yarn rw test" or "start": "cross-env NODE_OPTIONS=--experimental-specifier-resolution=node yarn rw dev", and the node options are consumed properly. But unfortunately it still fails. Both scripts cannot consume modules that are ESM only.

It would be great, to have at least a workaround.
I had similar issues in an old repo and at least for testing it was super convenient to use vitest - it just works.

If there is anything I can help with, please let me know, what I could do. I.e. set up a repo for this issue.

[ebramanti]

Is there any update on this issue? I am noticing more and more projects migrating to ESM and seems like this will only get more important to solve.

[maxall41]

I'm running into the same thing. Quite a lot of modules are now ESM and it's becoming a bit hard to work around this issue. Are there any workarounds to get this to work? Or will support for ESM modules be added in the next major revision of redwoodjs?

[dennemark]

Running into this issue again...
All @thi.ng/umbrella libraries are esm and therefore not useable :(

It might relate to this issue, that was unfortnately closed in typescript repo: microsoft/TypeScript#43329

typescript transpiles import to require statement, which does not work for es-only modules.

[jtoar]

Hey all, ESM support is something we're actively working on. We don't have a timetable yet, but we'll give a better update after the conference next week.

[taivo]

Hi. Just ran into this issue for @headlessui/react, although for the time being, I think the error can be ignored. Would love to see this moved forward. As in the case of jest being problematic, is switching to vitest being considered?

[dennemark]

Here is a workaround that might suit you in some cases. In my case I am importing tinypool. The workaround is currently needed, since typescript compiles an import to a require function although it should be import. And by creating a dynamic import function with a string, it wont be compiled. Solution can be derived also from similar approaches in the aforementioned issue (microsoft/TypeScript#43329)

export const importDynamic = new Function('modulePath', 'return import(modulePath)');

async function myFunction {
  const { Tinypool } = await importDynamic('tinypool');
  const pool = new Tinypool(...)
}

Only issue though: it is async.

[dennemark]

Typescript 5.3. might be helpful, too, soon: https://devblogs.microsoft.com/typescript/announcing-typescript-5-3/#stable-support-resolution-mode-in-import-types

// Resolve `pkg` as if we were importing with a `require()`
import type { TypeFromRequire } from "pkg" with {
    "resolution-mode": "require"
};

// Resolve `pkg` as if we were importing with an `import`
import type { TypeFromImport } from "pkg" with {
    "resolution-mode": "import"
};

[cjreimer]

Hello @jtoar, is there any update on this? We're running into this more and more often where libraries throw errors such as:

The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("compromise")' call instead.

The workaround, as suggested above, is typically to write a dynamic import, but shouldn't we support ECMAScript?

Thanks!

[Josh-Walker-GM]

Hey @cjreimer, always awesome to hear from you!

It's something that was on our radar but slipped down in priority a little lately so hasn't been moved forward a lot. Jtoar did do some solid work in this direction though!

Right now we can switch a redwood project over to ESM but we break a certain set of core features - the ones that come to mind right now are jest tests and exec scripts but there are likely more. We have a list somewhere but it escapes me right now.

This is work I have been really wanting to get back to and I will bring it up with the team next week to try to get some time scheduled to push it forward again.

What we could do is release an experimental setup command which would switch a project over to ESM. If we could have great users like yourself with large mature projects test it out and feedback on what breaks then it could help us out. We'll probably have to address a few core breaking changes before we can though - tests especially.

This turned into a bit of an info vomit... Is any of this helpful? I know there might not be anything actionable right now for you. Even just the pressure from users wanting this could help me argue for the time it needs.

[cjreimer]

Thanks for the quick response @Josh-Walker-GM !

Yes, I'd appreciate it if this ESM support could be bumped. The dynamic import isn't too bad, but the typescript types with dynamic imports can really be a pain!

I've played with converting to ESM on the web side.... and I've noted that it sort of works, but jest tests are impacted. I forget at the moment, but I think you are right that there are some other things impeding on the api side.

And yes, I would be very open to helping test out the changes. I appreciate the work on all this!

[larsvaehrens]

We would really appreciate this feature as well. We're counting over 100+ await import() in our services.

Also willing to help test changes on our end.

[Tobbe]

We're counting over 100+ await import() in our services.

Ouch 🙁

Josh did spend some more time on this after the nudge from both of you, so please do know that it's helpful and appreciated when you all keep us honest by commenting on issues like this 🙂

Latest related PR to go is was #10674
And we have this one in the works #10362

[larsvaehrens]

We're counting over 100+ await import() in our services.

Ouch 🙁

It's not as bad as it sounds 😅 We have a few packages in packages/ and one of them shares a lot of code between web, api, and the other packages, so that's where all the dynamic imports come from.

Josh did spend some more time on this after the nudge from both of you, so please do know that it's helpful and appreciated when you all keep us honest by commenting on issues like this 🙂

Appreciate the work and look forward to testing. We also use Vitest for our packages.

[twodotsmax]

@Josh-Walker-GM pls

Opentelemetry breaks and we can't practically use module: true without maybe breaking things