Skip to content

Commit

Permalink
fix(esm): use CJS wrapper for ESM default interop (#10119)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jtoar committed Mar 7, 2024
1 parent 6b81681 commit a85e0de
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 5 deletions.
10 changes: 7 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
# CHANGELOG

## Unreleased

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

This PR builds on the work started in https://github.com/redwoodjs/redwood/pull/10083 around ESM. One of the caveats of that PR was that the default export from `@redwoodjs/vite` broke. The workaround was referencing the `default` property on the Redwood Vite plugin, like `redwood.default()`. This fixes the ES module default export interoperability so that no change is necessary in switching between module types.

- feature: Enable [CSS nesting](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_nesting/Using_CSS_nesting) syntax by default when using Tailwind:

```
.button {
@apply p-2 font-semibold bg-gray-500;
Expand All @@ -17,8 +23,6 @@
}
```

## Unreleased

## v7.1.0

- See https://github.com/redwoodjs/redwood/releases/tag/v7.1.0
Expand Down
3 changes: 3 additions & 0 deletions packages/vite/cjsWrapper.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/* eslint-env node */

module.exports = require('./dist/index.js').default
5 changes: 3 additions & 2 deletions packages/vite/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"./package.json": "./package.json",
".": {
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
"default": "./cjsWrapper.js"
},
"./entries": {
"types": "./dist/entries.d.ts",
Expand Down Expand Up @@ -53,7 +53,8 @@
},
"files": [
"dist",
"inject"
"inject",
"cjsWrapper.js"
],
"scripts": {
"build": "yarn build:js && yarn build:types",
Expand Down

0 comments on commit a85e0de

Please sign in to comment.