cloudflare / workers-sdk

⛅️ Home to Wrangler, the CLI for Cloudflare Workers®
https://developers.cloudflare.com/workers/
Apache License 2.0
2.6k stars 675 forks source link

fix(wrangler): Watch for external dependencies changes in `pages dev` #6022

Closed CarmenPopoviciu closed 3 months ago

CarmenPopoviciu commented 3 months ago

What this PR solves / how to test

The watch mode in pages dev for Pages Functions projects is currently partially broken, as it only watches for file system changes in the /functions directory, but not for changes in any of the Functions' dependencies. This means that given a Pages Function math-is-fun.ts, defined as follows:

import { ADD } from "../math/add";

export async function onRequest() {
  return new Response(`${ADD} is fun!`);
}

pages dev will reload for any changes in math-is-fun.ts itself, but not for any changes in math/add.ts, which is its dependency.

Similarly, pages dev will not reload for any changes in non-JS module imports, such as wasm/html/binary module imports.

This PR fixes all these things, plus adds some extra polish to the pages dev watch mode experience.

Fixes #3840

Solutions we considered

Pages Functions projects cannot rely on esbuild's watch mode alone. That's because the watch mode that's built into esbuild is designed specifically for only triggering a rebuild when esbuild's build inputs are changed (see https://github.com/evanw/esbuild/issues/3705). With Functions, we would actually want to trigger a rebuild every time a new file is added to the "/functions" directory.

One solution would be to use an esbuild plugin, and "teach" esbuild to watch the "/functions" directory. But it's more complicated than that. When we build Functions, one of the steps is to generate the routes based on the file tree (see generateConfigFileFromTree). These routes are then injected into the esbuild entrypoint (see templates/pages-template-worker.ts). Delegating the "/functions" dir watching to an esbuild plugin, would mean delegating the routes generation to that plugin as well. This gets very hairy very quickly.

Another solution, is to use a combination of dependencies watching, via esbuild, and file system watching, via chokidar. The downside of this approach is that a lot of syncing between the two watch modes must be put in place, in order to avoid triggering building Functions multiple times over one single change (like for example renaming file that's part of the dependency graph).

Another solution, which is the one we opted for here, is to delegate file watching entirely to a file system watcher, chokidar in this case. While not entirely downside-free

this solution keeps all things watch mode in one place, makes things easier to read, reason about and maintain, separates Pages <-> esbuild concerns better, and gives all the flexibility we need.

How we tested the changes

This PR comes with e2e tests, but in addition to them, we've manually tested these changes in the following scenarios:

Author has addressed the following

changeset-bot[bot] commented 3 months ago

🦋 Changeset detected

Latest commit: 50aa43c459e456d7d752abe6e3f005f61121c7bc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages | Name | Type | | ------------------------------- | ----- | | wrangler | Patch | | @cloudflare/vitest-pool-workers | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions[bot] commented 3 months ago

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9759645322/npm-package-wrangler-6022

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/6022/npm-package-wrangler-6022

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9759645322/npm-package-wrangler-6022 dev path/to/script.js
Additional artifacts: ```sh npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9759645322/npm-package-create-cloudflare-6022 --no-auto-update ``` ```sh npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9759645322/npm-package-cloudflare-kv-asset-handler-6022 ``` ```sh npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9759645322/npm-package-miniflare-6022 ``` ```sh npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9759645322/npm-package-cloudflare-pages-shared-6022 ``` ```sh npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9759645322/npm-package-cloudflare-vitest-pool-workers-6022 ``` Note that these links will no longer work once [the GitHub Actions artifact expires](https://docs.github.com/en/organizations/managing-organization-settings/configuring-the-retention-period-for-github-actions-artifacts-and-logs-in-your-organization).

wrangler@3.62.0 includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20240620.0
workerd 1.20240620.1 1.20240620.1
workerd --version 1.20240620.1 2024-06-20

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

CarmenPopoviciu commented 3 months ago

thank you everyone for reviewing! It is much much appreciated <3. Merged! 🎉