File based dev registry #5214

penalosa · 2024-03-08T21:28:59Z

What this PR solves / how to test

Fixes #6037

Author has addressed the following

Tests
- TODO (before merge)
- Included
- Not necessary because: unskipped existing tests
Changeset (Changeset guidelines)
- TODO (before merge)
- Included
- Not necessary because:
Public documentation
- TODO (before merge)
- Cloudflare docs PR(s): https://github.com/cloudflare/cloudflare-docs/pull/...
- Not necessary because: not public facing—internal stability refactor gated behind a feature flag

changeset-bot · 2024-03-08T21:29:02Z

🦋 Changeset detected

Latest commit: 8d92959

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 · 2024-03-08T21:31:26Z

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/9585443653/npm-package-wrangler-5214

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

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

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9585443653/npm-package-wrangler-5214 dev path/to/script.js

Additional artifacts:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9585443653/npm-package-create-cloudflare-5214 --no-auto-update

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9585443653/npm-package-cloudflare-kv-asset-handler-5214

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9585443653/npm-package-miniflare-5214

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9585443653/npm-package-cloudflare-pages-shared-5214

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9585443653/npm-package-cloudflare-vitest-pool-workers-5214

Note that these links will no longer work once the GitHub Actions artifact expires.

wrangler@3.61.0 includes the following runtime dependencies:

Package	Constraint	Resolved
`miniflare`	workspace:*	3.20240610.1
`workerd`	1.20240610.1	1.20240610.1
`workerd --version`	1.20240610.1	2024-06-10

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

penalosa · 2024-03-11T16:08:28Z

Add file heartbeat to detect crashed workers
Add Wrangler version to files to detect back compat
Add cleanup mechanism when Wrangler tries to fetch a worker that no longer exists

RamIdeas · 2024-06-17T23:46:24Z

packages/wrangler/src/dev.tsx

+			.option("experimental-registry", {
+				alias: ["x-registry"],


RamIdeas · 2024-06-19T10:16:36Z

packages/wrangler/src/experimental-flags.ts

+export const run = <V>(flagValues: ExperimentalFlags, cb: () => V) =>
+	flags.run(flagValues, cb);
+
+export const FILE_BASED_REGISTRY = () => flags.getStore()?.FILE_BASED_REGISTRY;
+
+// TODO: actually use this
+export const DEV_ENV = () => flags.getStore()?.DEV_ENV;


Can we discuss alternatives to this interface please?

Absolutely—what are your concerns?

Minor point about interface:

I'd like to make adding a feature flag as easy as adding it to a type with no repetition. Having an exported function per flag requires multiple changes to this file to add each flag. While this allows us to find all usages of each feature flag with Find All References, they won't be easy to find usages of any flag. Can we try something like export const XFLAG = <F extends keyof ExperimentalFlags>(flag: F) => ExperimentalFlags[F];?

More for discussion:

This is a great use of AsyncLocalStorage. Can we generalise this use for other singletons like Logger etc? Are there any parts of wrangler this wouldn't work for?

petebacondarwin

Didn't have time to review it. But I see that @RamIdeas has approved.

petebacondarwin · 2024-06-18T07:01:00Z

fixtures/ai-app/tests/index.test.ts

@@ -27,8 +27,7 @@ describe("'wrangler dev' correctly renders pages", () => {
 		expect((content as Record<string, object>).run).toEqual("function");
 	});

-	// TODO: unskip when https://github.com/cloudflare/workerd/pull/2095 is merged and released
-	it.skip("ai binding properties", async ({ expect }) => {


petebacondarwin · 2024-06-18T07:02:02Z

fixtures/external-durable-objects-app/tests/index.test.ts

 			});
+			await setTimeout(1000);


Why the need for these pauses? If this is a shortcoming in the file-based reg approach, perhaps it is not ideal?

Discussed offline. This is a limitation to how unstable_dev works right now, not related to this refactor.
The problem is that unstable_dev will only read the dev-registry once, rather than polling it for changes.
So we need to make sure that upstream workers are in-place and ready before starting downstream workers.
The wrangler dev command does do polling, so does not have this problem. So normal users would not need such pauses.

Going forward we should add polling or watching to the unstable_dev stuff so that these are no longer needed.

mrbbot mentioned this pull request Mar 27, 2024

Add support for named entrypoints #5215

Merged

9 tasks

petebacondarwin mentioned this pull request Jun 17, 2024

🐛 BUG: Worker Registry fails to start(concurrent dev sessions) #6037

Closed

penalosa added 2 commits June 17, 2024 18:38

Implement file-based registry

e1e306e

cleanup + heartbeat

d9913f5

penalosa force-pushed the penalosa/file-based-dev-registry branch from dd3e0ce to d9913f5 Compare June 17, 2024 17:59

penalosa added 5 commits June 17, 2024 21:26

fix: support service worker syntax bindings locally

8c60a9c

enable & fix more tests

7bc84ea

another one bites the dust

e7f67f7

more unskipping

0feb82a

Create yellow-coins-serve.md

201ca70

penalosa marked this pull request as ready for review June 17, 2024 22:33

penalosa requested review from a team as code owners June 17, 2024 22:33

RamIdeas reviewed Jun 17, 2024

View reviewed changes

packages/wrangler/src/dev.tsx

Comment on lines +299 to +300

.option("experimental-registry", {

alias: ["x-registry"],

Copy link

Contributor

RamIdeas Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

RamIdeas reviewed Jun 19, 2024

View reviewed changes

Use dynamic flag lookup

76377c3

RamIdeas approved these changes Jun 19, 2024

View reviewed changes

petebacondarwin reviewed Jun 19, 2024

View reviewed changes

fix tests

8d92959

penalosa merged commit 05c5607 into main Jun 19, 2024
19 checks passed

penalosa deleted the penalosa/file-based-dev-registry branch June 19, 2024 16:44

workers-devprod mentioned this pull request Jun 19, 2024

Version Packages #6081

Merged

petebacondarwin mentioned this pull request Jul 1, 2024

🐛 BUG: wrangler dev -> fetch failed #3121

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

File based dev registry #5214

File based dev registry #5214

penalosa commented Mar 8, 2024 •

edited by RamIdeas

Loading

changeset-bot bot commented Mar 8, 2024 •

edited

Loading

github-actions bot commented Mar 8, 2024 •

edited

Loading

penalosa commented Mar 11, 2024 •

edited

Loading

RamIdeas Jun 17, 2024

RamIdeas Jun 19, 2024

penalosa Jun 19, 2024

RamIdeas Jun 19, 2024

petebacondarwin left a comment

petebacondarwin Jun 18, 2024

petebacondarwin Jun 18, 2024

petebacondarwin Jun 19, 2024

File based dev registry #5214

File based dev registry #5214

Conversation

penalosa commented Mar 8, 2024 • edited by RamIdeas Loading

What this PR solves / how to test

Author has addressed the following

changeset-bot bot commented Mar 8, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Mar 8, 2024 • edited Loading

penalosa commented Mar 11, 2024 • edited Loading

RamIdeas Jun 17, 2024

Choose a reason for hiding this comment

RamIdeas Jun 19, 2024

Choose a reason for hiding this comment

penalosa Jun 19, 2024

Choose a reason for hiding this comment

RamIdeas Jun 19, 2024

Choose a reason for hiding this comment

petebacondarwin left a comment

Choose a reason for hiding this comment

petebacondarwin Jun 18, 2024

Choose a reason for hiding this comment

petebacondarwin Jun 18, 2024

Choose a reason for hiding this comment

petebacondarwin Jun 19, 2024

Choose a reason for hiding this comment

penalosa commented Mar 8, 2024 •

edited by RamIdeas

Loading

changeset-bot bot commented Mar 8, 2024 •

edited

Loading

github-actions bot commented Mar 8, 2024 •

edited

Loading

penalosa commented Mar 11, 2024 •

edited

Loading