From a85e0ded7ab41dba57bea940bac74a3ee7b726f3 Mon Sep 17 00:00:00 2001 From: Dominic Saadi Date: Thu, 7 Mar 2024 10:10:03 -0800 Subject: [PATCH] fix(esm): use CJS wrapper for ESM default interop (#10119) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR builds on the work started in https://github.com/redwoodjs/redwood/pull/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 https://github.com/evanw/esbuild/issues/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. --- CHANGELOG.md | 10 +++++++--- packages/vite/cjsWrapper.js | 3 +++ packages/vite/package.json | 5 +++-- 3 files changed, 13 insertions(+), 5 deletions(-) create mode 100644 packages/vite/cjsWrapper.js diff --git a/CHANGELOG.md b/CHANGELOG.md index cf47b7b813e2..46c1f3257db5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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; @@ -17,8 +23,6 @@ } ``` -## Unreleased - ## v7.1.0 - See https://github.com/redwoodjs/redwood/releases/tag/v7.1.0 diff --git a/packages/vite/cjsWrapper.js b/packages/vite/cjsWrapper.js new file mode 100644 index 000000000000..c73b42a1198c --- /dev/null +++ b/packages/vite/cjsWrapper.js @@ -0,0 +1,3 @@ +/* eslint-env node */ + +module.exports = require('./dist/index.js').default diff --git a/packages/vite/package.json b/packages/vite/package.json index 0b1f4e6a1b3a..20cca5618294 100644 --- a/packages/vite/package.json +++ b/packages/vite/package.json @@ -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", @@ -53,7 +53,8 @@ }, "files": [ "dist", - "inject" + "inject", + "cjsWrapper.js" ], "scripts": { "build": "yarn build:js && yarn build:types",