Skip to content

Commit

Permalink
fix node.js compat module resolution (#8118)
Browse files Browse the repository at this point in the history
* fix node.js compat module resolution

Resolves #8108

* add e2e tests for node-compat

* fix linting issues

* post-review fixups

* update changeset
  • Loading branch information
petebacondarwin authored Feb 13, 2025
1 parent 35504e9 commit ca3cbc4
Show file tree
Hide file tree
Showing 33 changed files with 810 additions and 74 deletions.
21 changes: 21 additions & 0 deletions .changeset/wild-days-jog.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
"@cloudflare/vite-plugin": patch
---

fix Node.js compat module resolution

In v0.0.8 we landed support for Vite 6.1 and also switched to using the new Cloudflare owned unenv preset.
Unfortunately, the changes made in that update caused a regression in Node.js support.
This became apparent only when the plugin was being used with certain package managers and outside of the workers-sdk monorepo.

The unenv polyfills that get compiled into the Worker are transitive dependencies of this plugin, not direct dependencies of the user's application were the plugin is being used.
This is on purpose to avoid the user having to install these dependencies themselves.

Unfortunately, the changes in 0.0.8 did not correctly resolve the polyfills from `@cloudflare/unenv-preset` and `unenv` when the dependencies were not also installed directly into the user's application.

The approach was incorrectly relying upon setting the `importer` in calls to Vite's `resolve(id, importer)` method to base the resolution in the context of the vite plugin package rather than the user's application.
This doesn't work because the `importer` is only relevant when the `id` is relative, and not a bare module specifier in the case of the unenv polyfills.

This change fixes how these id are resolved in the plugin by manually resolving the path at the appropriate point, while still leveraging Vite's resolution pipeline to handle aliasing, and dependency optimization.

This change now introduces e2e tests that checks that isolated installations of the plugin works with npm, pnpm and yarn.
41 changes: 41 additions & 0 deletions packages/vite-plugin-cloudflare/e2e/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# vite-plugin e2e tests

This directory contains e2e test that give more confidence that the plugin will work in real world scenarios outside the comfort of this monorepo.

In general, these tests create test projects by copying a fixture from the `fixtures` directory into a temporary directory and then installing the local builds of the plugin along with its dependencies.

## Running the tests

Simply use turbo to run the tests from the root of the monorepo.
This will also ensure that the required dependencies have all been built before running the tests.

```sh
pnpm test:e2e -F @cloudflare/vite-plugin
```

## Developing e2e tests

These tests use a mock npm registry where the built plugin has been published.

The registry is booted up and loaded with the local build of the plugin and its local dependencies in the global-setup.ts file that runs once at the start of the e2e test run, and the server is killed and its caches removed at the end of the test run.

The Vite `test` function is an extended with additional helpers to setup clean copies of fixtures outside of the monorepo so that they can be isolated from any other dependencies in the project.

The simplest test looks like:

```ts
test("can serve a Worker request", async ({ expect, seed, viteDev }) => {
const projectPath = await seed("basic");
runCommand(`pnpm install`, projectPath);

const proc = await viteDev(projectPath);
const url = await waitForReady(proc);
expect(await fetchJson(url + "/api/")).toEqual({ name: "Cloudflare" });
});
```

- The `seed()` helper makes a copy of the named fixture into a temporary directory. It returns the path to the directory containing the copy (`projectPath` above). This directory will be deleted at the end of the test.
- The `runCommand()` helper simply executes a one-shot command and resolves when it has exited. You can use this to install the dependencies of the fixture from the mock npm registry, as in the example above.
- The `viteDev()` helper boots up the `vite dev` command and returns an object that can be used to monitor its output. The process will be killed at the end of the test.
- The `waitForReady()` helper will resolve when the `vite dev` process has output its ready message, from which it will parse the url that can be fetched in the test.
- The `fetchJson()` helper makes an Undici fetch to the url parsing the response into JSON. It will retry every 250ms for up to 10 secs to minimize flakes.
15 changes: 15 additions & 0 deletions packages/vite-plugin-cloudflare/e2e/basic.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { describe } from "vitest";
import { fetchJson, runCommand, test, waitForReady } from "./helpers.js";

describe("node compatibility", () => {
describe.each(["pnpm", "npm", "yarn"])("using %s", (pm) => {
test("can serve a Worker request", async ({ expect, seed, viteDev }) => {
const projectPath = await seed("basic");
runCommand(`${pm} install`, projectPath);

const proc = await viteDev(projectPath);
const url = await waitForReady(proc);
expect(await fetchJson(url + "/api/")).toEqual({ name: "Cloudflare" });
});
});
});
24 changes: 24 additions & 0 deletions packages/vite-plugin-cloudflare/e2e/fixtures/basic/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Logs
logs
*.log
npm-debug.log*
yarn-debug.log*
yarn-error.log*
pnpm-debug.log*
lerna-debug.log*

node_modules
dist
dist-ssr
*.local

# Editor directories and files
.vscode/*
!.vscode/extensions.json
.idea
.DS_Store
*.suo
*.ntvs*
*.njsproj
*.sln
*.sw?
50 changes: 50 additions & 0 deletions packages/vite-plugin-cloudflare/e2e/fixtures/basic/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# React + TypeScript + Vite

This template provides a minimal setup to get React working in Vite with HMR and some ESLint rules.

Currently, two official plugins are available:

- [@vitejs/plugin-react](https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react/README.md) uses [Babel](https://babeljs.io/) for Fast Refresh
- [@vitejs/plugin-react-swc](https://github.com/vitejs/vite-plugin-react-swc) uses [SWC](https://swc.rs/) for Fast Refresh

## Expanding the ESLint configuration

If you are developing a production application, we recommend updating the configuration to enable type aware lint rules:

- Configure the top-level `parserOptions` property like this:

```js
export default tseslint.config({
languageOptions: {
// other options...
parserOptions: {
project: ["./tsconfig.node.json", "./tsconfig.app.json"],
tsconfigRootDir: import.meta.dirname,
},
},
});
```

- Replace `tseslint.configs.recommended` to `tseslint.configs.recommendedTypeChecked` or `tseslint.configs.strictTypeChecked`
- Optionally add `...tseslint.configs.stylisticTypeChecked`
- Install [eslint-plugin-react](https://github.com/jsx-eslint/eslint-plugin-react) and update the config:

```js
// eslint.config.js
import react from "eslint-plugin-react";

export default tseslint.config({
// Set the react version
settings: { react: { version: "18.3" } },
plugins: {
// Add the react plugin
react,
},
rules: {
// other rules...
// Enable its recommended rules
...react.configs.recommended.rules,
...react.configs["jsx-runtime"].rules,
},
});
```
17 changes: 17 additions & 0 deletions packages/vite-plugin-cloudflare/e2e/fixtures/basic/api/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
interface Env {
ASSETS: Fetcher;
}

export default {
fetch(request, env) {
const url = new URL(request.url);

if (url.pathname.startsWith("/api/")) {
return Response.json({
name: "Cloudflare",
});
}

return env.ASSETS.fetch(request);
},
} satisfies ExportedHandler<Env>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import js from "@eslint/js";
import reactHooks from "eslint-plugin-react-hooks";
import reactRefresh from "eslint-plugin-react-refresh";
import globals from "globals";
import tseslint from "typescript-eslint";

export default tseslint.config(
{ ignores: ["dist"] },
{
extends: [js.configs.recommended, ...tseslint.configs.recommended],
files: ["**/*.{ts,tsx}"],
languageOptions: {
ecmaVersion: 2020,
globals: globals.browser,
},
plugins: {
"react-hooks": reactHooks,
"react-refresh": reactRefresh,
},
rules: {
...reactHooks.configs.recommended.rules,
"react-refresh/only-export-components": [
"warn",
{ allowConstantExport: true },
],
},
}
);
13 changes: 13 additions & 0 deletions packages/vite-plugin-cloudflare/e2e/fixtures/basic/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<link rel="icon" type="image/svg+xml" href="/vite.svg" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Vite + React + TS</title>
</head>
<body>
<div id="root"></div>
<script type="module" src="/src/main.tsx"></script>
</body>
</html>
32 changes: 32 additions & 0 deletions packages/vite-plugin-cloudflare/e2e/fixtures/basic/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"name": "cloudflare-vite-tutorial",
"version": "0.0.0",
"private": true,
"type": "module",
"scripts": {
"build": "tsc -b && vite build",
"dev": "vite",
"lint": "eslint .",
"preview": "vite preview"
},
"dependencies": {
"react": "^19.0.0",
"react-dom": "^19.0.0"
},
"devDependencies": {
"@cloudflare/vite-plugin": "https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13293829928/npm-package-cloudflare-vite-plugin-8118",
"@cloudflare/workers-types": "^4.20250204.0",
"@eslint/js": "^9.19.0",
"@types/react": "^19.0.8",
"@types/react-dom": "^19.0.3",
"@vitejs/plugin-react": "^4.3.4",
"eslint": "^9.19.0",
"eslint-plugin-react-hooks": "^5.0.0",
"eslint-plugin-react-refresh": "^0.4.18",
"globals": "^15.14.0",
"typescript": "~5.7.2",
"typescript-eslint": "^8.22.0",
"vite": "^6.1.0",
"wrangler": "^3.108.1"
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
42 changes: 42 additions & 0 deletions packages/vite-plugin-cloudflare/e2e/fixtures/basic/src/App.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#root {
max-width: 1280px;
margin: 0 auto;
padding: 2rem;
text-align: center;
}

.logo {
height: 6em;
padding: 1.5em;
will-change: filter;
transition: filter 300ms;
}
.logo:hover {
filter: drop-shadow(0 0 2em #646cffaa);
}
.logo.react:hover {
filter: drop-shadow(0 0 2em #61dafbaa);
}

@keyframes logo-spin {
from {
transform: rotate(0deg);
}
to {
transform: rotate(360deg);
}
}

@media (prefers-reduced-motion: no-preference) {
a:nth-of-type(2) .logo {
animation: logo-spin infinite 20s linear;
}
}

.card {
padding: 2em;
}

.read-the-docs {
color: #888;
}
56 changes: 56 additions & 0 deletions packages/vite-plugin-cloudflare/e2e/fixtures/basic/src/App.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// src/App.tsx

import viteLogo from "/vite.svg";
import { useState } from "react";
import reactLogo from "./assets/react.svg";
import "./App.css";

function App() {
const [count, setCount] = useState(0);
const [name, setName] = useState("unknown");

return (
<>
<div>
<a href="https://vite.dev" target="_blank">
<img src={viteLogo} className="logo" alt="Vite logo" />
</a>
<a href="https://react.dev" target="_blank">
<img src={reactLogo} className="logo react" alt="React logo" />
</a>
</div>
<h1>Vite + React</h1>
<div className="card">
<button
onClick={() => setCount((count) => count + 1)}
aria-label="increment"
>
count is {count}
</button>
<p>
Edit <code>src/App.tsx</code> and save to test HMR
</p>
</div>
<div className="card">
<button
onClick={() => {
fetch("/api/")
.then((res) => res.json() as Promise<{ name: string }>)
.then((data) => setName(data.name));
}}
aria-label="get name"
>
Name from API is: {name}
</button>
<p>
Edit <code>api/index.ts</code> to change the name
</p>
</div>
<p className="read-the-docs">
Click on the Vite and React logos to learn more
</p>
</>
);
}

export default App;
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading

0 comments on commit ca3cbc4

Please sign in to comment.