fix(esm): fix initial esm blockers for redwood apps #10083

jtoar · 2024-03-01T00:24:40Z

This PR makes some initial fixes that were required for making a Redwood app ESM. @Josh-Walker-GM and I tested by making a new project, specifying "type": "module" in all the package.jsons, and trying to get things to work (namely, yarn rw build and yarn rw serve). We'll ramp up our testing to one of the test project fixtures after we get these initial things fixed. Everything here is backwards-compatible.

Some of the changes we had to make were in the project itself. Namely, specifying file extensions for relative imports (import { db } from 'src/lib/db.js' instead of import { db } from 'src/lib/db'). But others needed to be made at the framework level:

esbuild has to emit ESM
packages/api-server/src/plugins/lambdaLoader.ts can't use require to import an ES module
Babel plugins have to emit relative file imports with extensions
The @redwoodjs/graphql-server package needs to be built differently to avoid having to do this...
```
// ./api/src/functions/graphql.ts
import pkg from '@redwoodjs/graphql-server'
const { createGraphQLHandler } = pkg
```
Not 100% sure why, but Babel makes some exports getters instead of just exporting them statically. Better to have less magic going on in dist.

Some manual changes to a project are inevitable (the file extensions), but we'd like to minimize them as much as possible. Here's a few that were necessary that we haven't figured out yet...

dist imports, like import ... from '@redwoodjs/api/logger', don't work

They need to be changed to import ... from '@redwoodjs/api/logger/index.js'. It feels like the only way to really fix this one is to use exports in the package.json, but if I remember correctly, that requires tsconfig settings that requires ESM

redwood is not a function in ./web/vite.config.ts:

We haven't gotten to the bottom of this one yet:

failed to load config from ~/redwood/redwood-app-esm/web/vite.config.mts
file:///~/redwood/redwood-app-esm/web/vite.config.mts.timestamp-1709251612918- 
6f6a7f8efad05.mjs:7
  plugins: [redwood()]
            ^

TypeError: redwood is not a function
    at file:///~/redwood/redwood-app-esm/web/vite.config.mts.timestamp-1709251612918- 
6f6a7f8efad05.mjs:7:13

The fix is accessing redwood.default:

@Josh-Walker-GM

This PR makes some initial fixes that were required for making a Redwood app ESM. @Josh-Walker-GM and I tested by making a new project, specifying `"type": "module"` in all the package.jsons, and trying to get things to work (namely, `yarn rw build` and `yarn rw serve`). We'll ramp up our testing to one of the test project fixtures after we get these initial things fixed. Everything here is backwards-compatible. Some of the changes we had to make were in the project itself. Namely, specifying file extensions for relative imports (`import { db } from 'src/lib/db.js'` instead of `import { db } from 'src/lib/db'`). But others needed to be made at the framework level: - esbuild has to emit ESM - `packages/api-server/src/plugins/lambdaLoader.ts` can't use `require` to import an ES module - Babel plugins have to emit relative file imports with extensions - The `@redwoodjs/graphql-server` package needs to be built differently to avoid having to do this... ```ts // ./api/src/functions/graphql.ts import pkg from '@redwoodjs/graphql-server' const { createGraphQLHandler } = pkg ``` Not 100% sure why, but Babel makes some exports getters instead of just exporting them statically. Better to have less magic going on in dist. Some manual changes to a project are inevitable (the file extensions), but we'd like to minimize them as much as possible. Here's a few that were necessary that we haven't figured out yet... - dist imports, like `import ... from '@redwoodjs/api/logger'`, don't work They need to be changed to `import ... from '@redwoodjs/api/logger/index.js'`. It feels like the only way to really fix this one is to use `exports` in the package.json, but if I remember correctly, that requires tsconfig settings that requires ESM - `redwood` is not a function in `./web/vite.config.ts`: We haven't gotten to the bottom of this one yet: ``` failed to load config from ~/redwood/redwood-app-esm/web/vite.config.mts file:///~/redwood/redwood-app-esm/web/vite.config.mts.timestamp-1709251612918- 6f6a7f8efad05.mjs:7 plugins: [redwood()] ^ TypeError: redwood is not a function at file:///~/redwood/redwood-app-esm/web/vite.config.mts.timestamp-1709251612918- 6f6a7f8efad05.mjs:7:13 ``` The fix is accessing `redwood.default`: <img width="685" alt="image" src="https://github.com/redwoodjs/redwood/assets/32992335/4f09fea4-42d3-49a4-9339-74b42c90621e">

@Josh-Walker-GM

This PR makes some initial fixes that were required for making a Redwood app ESM. @Josh-Walker-GM and I tested by making a new project, specifying `"type": "module"` in all the package.jsons, and trying to get things to work (namely, `yarn rw build` and `yarn rw serve`). We'll ramp up our testing to one of the test project fixtures after we get these initial things fixed. Everything here is backwards-compatible. Some of the changes we had to make were in the project itself. Namely, specifying file extensions for relative imports (`import { db } from 'src/lib/db.js'` instead of `import { db } from 'src/lib/db'`). But others needed to be made at the framework level: - esbuild has to emit ESM - `packages/api-server/src/plugins/lambdaLoader.ts` can't use `require` to import an ES module - Babel plugins have to emit relative file imports with extensions - The `@redwoodjs/graphql-server` package needs to be built differently to avoid having to do this... ```ts // ./api/src/functions/graphql.ts import pkg from '@redwoodjs/graphql-server' const { createGraphQLHandler } = pkg ``` Not 100% sure why, but Babel makes some exports getters instead of just exporting them statically. Better to have less magic going on in dist. Some manual changes to a project are inevitable (the file extensions), but we'd like to minimize them as much as possible. Here's a few that were necessary that we haven't figured out yet... - dist imports, like `import ... from '@redwoodjs/api/logger'`, don't work They need to be changed to `import ... from '@redwoodjs/api/logger/index.js'`. It feels like the only way to really fix this one is to use `exports` in the package.json, but if I remember correctly, that requires tsconfig settings that requires ESM - `redwood` is not a function in `./web/vite.config.ts`: We haven't gotten to the bottom of this one yet: ``` failed to load config from ~/redwood/redwood-app-esm/web/vite.config.mts file:///~/redwood/redwood-app-esm/web/vite.config.mts.timestamp-1709251612918- 6f6a7f8efad05.mjs:7 plugins: [redwood()] ^ TypeError: redwood is not a function at file:///~/redwood/redwood-app-esm/web/vite.config.mts.timestamp-1709251612918- 6f6a7f8efad05.mjs:7:13 ``` The fix is accessing `redwood.default`: <img width="685" alt="image" src="https://github.com/redwoodjs/redwood/assets/32992335/4f09fea4-42d3-49a4-9339-74b42c90621e">

@Tobbe

This PR builds on the work started in #10083. It does two things: - ~builds the `@redwoodjs/vite` package with esbuild instead of babel (this is optional but just did it to debug, but I'd imagine we'd want to start doing this anyway)~ - CI wasn't happy with this change so removed for now - introduces a CJS wrapper to handle ESM export default interoperation in Node To frame this change at a high level, one of the ways I'm approaching ESM is backwards from Redwood apps. If Redwood apps can be ESM (via "type": "module" in package.jsons, updates to tsconfigs, adding file extensions to relative imports, etc), then we may have an easier time rolling out packages that can't be shipped as dual ESM/CJS exports. (Vite is probably one of them. But since Vite evaluates the config file itself, it may be ok.) One of the framework-level bugs that shows up when you make an app ESM is this error: ``` failed to load config from ~/redwood/redwood-app-esm/web/vite.config.mts file:///~/redwood/redwood-app-esm/web/vite.config.mts.timestamp-1709251612918- 6f6a7f8efad05.mjs:7 plugins: [redwood()] ^ TypeError: redwood is not a function at file:///~/redwood/redwood-app-esm/web/vite.config.mts.timestamp-1709251612918- 6f6a7f8efad05.mjs:7:13 ``` The culprit is this import in `web/vite.config.ts`: ```ts import redwood from '@redwoodjs/vite' ``` The problem is confusing, but basically... when you 1) transpile an ES module into CJS (which is what we're doing for most of our packages) and then 2) import that CJS module from a bona fide ES module, the default export doesn't behave how you'd expect. Evan Wallace has a great write up of it [here](https://esbuild.github.io/content-types/#default-interop). (For Node's official docs, see https://nodejs.org/docs/latest/api/esm.html#commonjs-namespaces.) As a potential fix, @Tobbe pointed me to this Vite plugin: https://github.com/cyco130/vite-plugin-cjs-interop. I tried adding it but the issue is that this **is** the Vite config file. 😅 But I thought I may be able to add the plugin here when we call `createServer`: https://github.com/redwoodjs/redwood/blob/e9ecbb07da216210c59a1e499816f31c025fe81d/packages/vite/bins/rw-vite-dev.mjs#L28-L37 But it still wasn't working. I stepped through the source a bit and it doesn't seem like there's room for configuring how Vite loads the config file. The two main functions were these, `loadConfigFromBundledFile` and `loadConfigFromFile`: - https://github.com/vitejs/vite/blob/e92abe58164682c2e468318c05023bfb4ecdfa02/packages/vite/src/node/config.ts#L1186 - https://github.com/vitejs/vite/blob/e92abe58164682c2e468318c05023bfb4ecdfa02/packages/vite/src/node/config.ts#L962 `bundleConfigFile` has plugins configured, but they're hardcoded. But luckily I don't think it's necessary... based on this esbuild thread, it seems like we can get away with a small wrapper. See evanw/esbuild#532. When we actually make this an ES module (which will happen pretty soon I think), this will go away. But for the time being, this is a CJS module.

@Tobbe

This PR builds on the work started in #10083. It does two things: - ~builds the `@redwoodjs/vite` package with esbuild instead of babel (this is optional but just did it to debug, but I'd imagine we'd want to start doing this anyway)~ - CI wasn't happy with this change so removed for now - introduces a CJS wrapper to handle ESM export default interoperation in Node To frame this change at a high level, one of the ways I'm approaching ESM is backwards from Redwood apps. If Redwood apps can be ESM (via "type": "module" in package.jsons, updates to tsconfigs, adding file extensions to relative imports, etc), then we may have an easier time rolling out packages that can't be shipped as dual ESM/CJS exports. (Vite is probably one of them. But since Vite evaluates the config file itself, it may be ok.) One of the framework-level bugs that shows up when you make an app ESM is this error: ``` failed to load config from ~/redwood/redwood-app-esm/web/vite.config.mts file:///~/redwood/redwood-app-esm/web/vite.config.mts.timestamp-1709251612918- 6f6a7f8efad05.mjs:7 plugins: [redwood()] ^ TypeError: redwood is not a function at file:///~/redwood/redwood-app-esm/web/vite.config.mts.timestamp-1709251612918- 6f6a7f8efad05.mjs:7:13 ``` The culprit is this import in `web/vite.config.ts`: ```ts import redwood from '@redwoodjs/vite' ``` The problem is confusing, but basically... when you 1) transpile an ES module into CJS (which is what we're doing for most of our packages) and then 2) import that CJS module from a bona fide ES module, the default export doesn't behave how you'd expect. Evan Wallace has a great write up of it [here](https://esbuild.github.io/content-types/#default-interop). (For Node's official docs, see https://nodejs.org/docs/latest/api/esm.html#commonjs-namespaces.) As a potential fix, @Tobbe pointed me to this Vite plugin: https://github.com/cyco130/vite-plugin-cjs-interop. I tried adding it but the issue is that this **is** the Vite config file. 😅 But I thought I may be able to add the plugin here when we call `createServer`: https://github.com/redwoodjs/redwood/blob/e9ecbb07da216210c59a1e499816f31c025fe81d/packages/vite/bins/rw-vite-dev.mjs#L28-L37 But it still wasn't working. I stepped through the source a bit and it doesn't seem like there's room for configuring how Vite loads the config file. The two main functions were these, `loadConfigFromBundledFile` and `loadConfigFromFile`: - https://github.com/vitejs/vite/blob/e92abe58164682c2e468318c05023bfb4ecdfa02/packages/vite/src/node/config.ts#L1186 - https://github.com/vitejs/vite/blob/e92abe58164682c2e468318c05023bfb4ecdfa02/packages/vite/src/node/config.ts#L962 `bundleConfigFile` has plugins configured, but they're hardcoded. But luckily I don't think it's necessary... based on this esbuild thread, it seems like we can get away with a small wrapper. See evanw/esbuild#532. When we actually make this an ES module (which will happen pretty soon I think), this will go away. But for the time being, this is a CJS module.

@Tobbe

This PR builds on the work started in #10083. It does two things: - ~builds the `@redwoodjs/vite` package with esbuild instead of babel (this is optional but just did it to debug, but I'd imagine we'd want to start doing this anyway)~ - CI wasn't happy with this change so removed for now - introduces a CJS wrapper to handle ESM export default interoperation in Node To frame this change at a high level, one of the ways I'm approaching ESM is backwards from Redwood apps. If Redwood apps can be ESM (via "type": "module" in package.jsons, updates to tsconfigs, adding file extensions to relative imports, etc), then we may have an easier time rolling out packages that can't be shipped as dual ESM/CJS exports. (Vite is probably one of them. But since Vite evaluates the config file itself, it may be ok.) One of the framework-level bugs that shows up when you make an app ESM is this error: ``` failed to load config from ~/redwood/redwood-app-esm/web/vite.config.mts file:///~/redwood/redwood-app-esm/web/vite.config.mts.timestamp-1709251612918- 6f6a7f8efad05.mjs:7 plugins: [redwood()] ^ TypeError: redwood is not a function at file:///~/redwood/redwood-app-esm/web/vite.config.mts.timestamp-1709251612918- 6f6a7f8efad05.mjs:7:13 ``` The culprit is this import in `web/vite.config.ts`: ```ts import redwood from '@redwoodjs/vite' ``` The problem is confusing, but basically... when you 1) transpile an ES module into CJS (which is what we're doing for most of our packages) and then 2) import that CJS module from a bona fide ES module, the default export doesn't behave how you'd expect. Evan Wallace has a great write up of it [here](https://esbuild.github.io/content-types/#default-interop). (For Node's official docs, see https://nodejs.org/docs/latest/api/esm.html#commonjs-namespaces.) As a potential fix, @Tobbe pointed me to this Vite plugin: https://github.com/cyco130/vite-plugin-cjs-interop. I tried adding it but the issue is that this **is** the Vite config file. 😅 But I thought I may be able to add the plugin here when we call `createServer`: https://github.com/redwoodjs/redwood/blob/e9ecbb07da216210c59a1e499816f31c025fe81d/packages/vite/bins/rw-vite-dev.mjs#L28-L37 But it still wasn't working. I stepped through the source a bit and it doesn't seem like there's room for configuring how Vite loads the config file. The two main functions were these, `loadConfigFromBundledFile` and `loadConfigFromFile`: - https://github.com/vitejs/vite/blob/e92abe58164682c2e468318c05023bfb4ecdfa02/packages/vite/src/node/config.ts#L1186 - https://github.com/vitejs/vite/blob/e92abe58164682c2e468318c05023bfb4ecdfa02/packages/vite/src/node/config.ts#L962 `bundleConfigFile` has plugins configured, but they're hardcoded. But luckily I don't think it's necessary... based on this esbuild thread, it seems like we can get away with a small wrapper. See evanw/esbuild#532. When we actually make this an ES module (which will happen pretty soon I think), this will go away. But for the time being, this is a CJS module.

initial esm work for rw apps

66c8f34

jtoar added the release:fix This PR is a fix label Mar 1, 2024

jtoar added this to the next-release-patch milestone Mar 1, 2024

update changelog

63750c2

jtoar marked this pull request as draft March 1, 2024 00:52

jtoar added 3 commits February 29, 2024 18:35

work on babel tests

3ac0795

remove test that ensured broken behavior

72bf023

fix api server tests

d24bf54

jtoar marked this pull request as ready for review March 1, 2024 02:47

use file urls for windows

4612e75

jtoar marked this pull request as draft March 1, 2024 03:32

jtoar added 5 commits February 29, 2024 21:21

refactor for prerender

1339ee5

make options not required

a24382f

add basic tests for esm helpers

95e3f07

fix for esm test project

3f6833a

update changelog

7d27d19

jtoar marked this pull request as ready for review March 1, 2024 05:54

fix data migrate test

b9b8aff

jtoar merged commit 7dc7f55 into main Mar 1, 2024
41 checks passed

jtoar deleted the ds-jgmw-esm/initial-esm-work-for-rw-apps branch March 1, 2024 06:23

ahaywood modified the milestones: next-release-patch, v7.0.6 Mar 1, 2024

jtoar mentioned this pull request Mar 6, 2024

fix(esm): use CJS wrapper for ESM default interop #10119

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(esm): fix initial esm blockers for redwood apps #10083

fix(esm): fix initial esm blockers for redwood apps #10083

jtoar commented Mar 1, 2024 •

edited

Loading

fix(esm): fix initial esm blockers for redwood apps #10083

fix(esm): fix initial esm blockers for redwood apps #10083

Conversation

jtoar commented Mar 1, 2024 • edited Loading

jtoar commented Mar 1, 2024 •

edited

Loading