Turn on `wrangler.json(c)` support by default #7230

penalosa · 2024-11-11T17:57:04Z

Turn on the --experimental-json-config flag by default, enabling reading of wrangler.json & wrangler.jsonc config files. It can still be disabled by setting --experimental-json-config=false. Since it's on by default, this also unblocks support for Pages (pending release of this PR & bumping of Wrangler in the build image)

This PR only enables it in Wrangler. Follow up PRs will adjust the documentation to call out this support properly as well as updating C3.

Tests
- TODO (before merge)
- Tests included
- Tests not necessary because:
E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
- I don't know
- Required
- Not required because:
Public documentation
- TODO (before merge)
- Cloudflare docs PR(s): Wrangler config updates cloudflare-docs#18313
- Documentation not necessary because:

changeset-bot · 2024-11-11T17:57:08Z

🦋 Changeset detected

Latest commit: 174a555

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

This PR includes changesets to release 2 packages

Name	Type
wrangler	Minor
@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-11-11T18:03:54Z

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/11975603642/npm-package-wrangler-7230

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

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

Or you can use npx with this latest build directly:

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

Additional artifacts:

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

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

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

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

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

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11975603642/npm-package-cloudflare-workers-editor-shared-7230

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11975603642/npm-package-cloudflare-workers-shared-7230

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11975603642/npm-package-cloudflare-workflows-shared-7230

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

[email protected] includes the following runtime dependencies:

Package	Constraint	Resolved
`miniflare`	workspace:*	3.20241106.1
`workerd`	1.20241106.1	1.20241106.1
`workerd --version`	1.20241106.1	2024-11-06

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

packages/wrangler/src/index.ts

petebacondarwin

Ran out of time so dumping my comments so far.

.changeset/brave-paws-build.md

petebacondarwin · 2024-11-21T09:30:45Z

packages/wrangler/src/__tests__/configuration.test.ts

-				      \`\`\`
+				    \`\`\`
+				    [[migrations]]
+				    tag = \\"v1\\"


Hmm, this change seems a bit out of place for this PR.

petebacondarwin · 2024-11-21T09:33:06Z

packages/wrangler/src/__tests__/deploy.test.ts

-			            Or you could pass it in your terminal as \`--compatibility-date XXXX-XX-XX\`
-			        See https://developers.cloudflare.com/workers/platform/compatibility-dates for more information."
-		      `);
+				"A compatibility_date is required when publishing. Add the following to your wrangler.toml file:.


Given the snippet below is now JSON I think this needs to be wrangler.json.

petebacondarwin · 2024-11-21T09:34:25Z

packages/wrangler/src/__tests__/deploy.test.ts

-			    - In wrangler.toml, you have configured [durable_objects] exported by this Worker (SomeClass),
-			  but no [migrations] for them. This may not work as expected until you add a [migrations] section
-			  to your wrangler.toml. Add this configuration to your wrangler.toml:
+				    - In wrangler.toml, you have configured [durable_objects] exported by this Worker (SomeClass),


Do we need to change these messages to accommodate the actual format of the config file in use?

packages/wrangler/src/__tests__/kv.test.ts

packages/wrangler/src/__tests__/pages/deploy.test.ts

petebacondarwin · 2024-11-21T09:47:56Z

packages/wrangler/src/config/config.ts

-export type Config = ConfigFields<DevConfig> & PagesConfigFields & Environment;
+export type Config = ConfigFields<DevConfig> &
+	PagesConfigFields &
+	Environment & { parsedFormat?: "jsonc" | "toml" };


I'm not keen on adding extra stuff inline here.
I note that there is already a precedent in that configPath is part of ConfigFields but is not something we can read from the wrangler.toml/json directly (despite appearing in RawConfig).

I think the most accurate way to do this is to create a new ConfigInfo type, which contains configPath and parsedFormat (perhaps configFormat?).
Then include this in the Config type (and not in the ConfigFields type) wyich will avoid it appearing in the RawConfig type.

petebacondarwin · 2024-11-21T09:49:23Z

packages/wrangler/src/config/index.ts

-	experimentalJsonConfig?: boolean | undefined;
-};
+export function formatConfigSnippet(
+	config: RawEnvironment,


Can we call this something like snippet to avoid the reader from thinking that we are passing in the current Wrangler configuration here?

petebacondarwin · 2024-11-21T09:50:20Z

packages/wrangler/src/config/index.ts

+export function formatConfigSnippet(
+	config: RawEnvironment,
+	parsedFormat: Config["parsedFormat"],
+	spacing = true


BIKESHED: formatted or pretty or something?

petebacondarwin

I love where this is going but the more I read through the changes the more I questioned whether we need a parseFormat property on config.
We have the configPath property already, and it would be simple to provide a helper function like:

function configFormat(configPath: string|undefined): "json"|"toml"|"none" { ... }

This means not having to thread this extra parameter around alongside the configPath.
Moreover, as you can guess from the signature of the helper function above, there is the scenario where the user has not provided a config file at all, which we need to support. In that case, snippet messaging might be more along the lines of "Create a wrangler.json with this content".

petebacondarwin · 2024-11-21T13:50:49Z

packages/wrangler/src/config/validation.ts

@@ -73,7 +73,8 @@ export function isPagesConfig(rawConfig: RawConfig): boolean {
 export function normalizeAndValidateConfig(
 	rawConfig: RawConfig,
 	configPath: string | undefined,
-	args: NormalizeAndValidateConfigArgs
+	args: NormalizeAndValidateConfigArgs,
+	parsedFormat: Config["parsedFormat"] = "toml"


This function is internal, so there is no value in making this field optional, IMO. It is better to constrain all usage to provide this. That also helps to ensure we have considered every usage.

Also since this and configPath are similar properties (in their usage) I would propose to group this new param as the third one ahead of args.

packages/wrangler/src/config/validation.ts

…rangler.json is used

petebacondarwin

Love it!
There may be some instances missing but I didn't find any.
Ship it!

packages/wrangler/README.md

petebacondarwin · 2024-11-21T20:53:56Z

packages/wrangler/src/__tests__/helpers/write-wrangler-config.ts

+) {
+	fs.writeFileSync(
+		path,
+		formatConfigSnippet(


Co-authored-by: Pete Bacon Darwin <[email protected]>

penalosa requested review from a team as code owners November 11, 2024 17:57

Skye-31 reviewed Nov 11, 2024

View reviewed changes

packages/wrangler/src/index.ts Outdated Show resolved Hide resolved

penalosa force-pushed the penalosa/json-no-flag branch from aeab1b9 to 61329eb Compare November 16, 2024 02:10

petebacondarwin reviewed Nov 21, 2024

View reviewed changes

petebacondarwin requested changes Nov 21, 2024

View reviewed changes

penalosa added 9 commits November 21, 2024 18:18

Turn on JSON by default

22afb75

fix pages snapshots

e9a12ab

Create brave-paws-build.md

b351648

Ensure Wrangler outputs TOML when wrangler.toml is used & JSON when w…

ab53819

…rangler.json is used

Hide -j flag

f926091

linting

a1cbe46

Rip out -j boilerplate

32f351a

snapshots

19b5e13

Rename more things

5d3a166

penalosa force-pushed the penalosa/json-no-flag branch from 217e8b4 to 5d3a166 Compare November 21, 2024 19:24

penalosa added 3 commits November 21, 2024 20:09

Fix tests

ba341ac

stray

25e1e79

fix tests

1bf7e03

petebacondarwin approved these changes Nov 21, 2024

View reviewed changes

penalosa and others added 4 commits November 21, 2024 21:43

Update packages/wrangler/README.md

721597c

Co-authored-by: Pete Bacon Darwin <[email protected]>

fix test typing

73396f2

windows

edee055

fix lint

eb39854

penalosa added the e2e Run e2e tests on a PR label Nov 21, 2024

update regex for json

8dd58a5

penalosa added 6 commits November 22, 2024 02:21

update regex for json

e490b6f

fix pages dev e2e

62e961e

Fix R2 snapshots

f9bf716

Update regex for json

c4efa08

fix r3

f0b2ff5

fix types e2e

174a555

penalosa merged commit 6fe9533 into main Nov 22, 2024
32 checks passed

penalosa deleted the penalosa/json-no-flag branch November 22, 2024 17:10

workers-devprod mentioned this pull request Nov 22, 2024

Version Packages #7325

Merged

penalosa mentioned this pull request Nov 25, 2024

Print copyable wrangler.toml text when creating a hyperdrive config #7326

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Turn on `wrangler.json(c)` support by default #7230

Turn on `wrangler.json(c)` support by default #7230

penalosa commented Nov 11, 2024 •

edited

Loading

changeset-bot bot commented Nov 11, 2024 •

edited

Loading

github-actions bot commented Nov 11, 2024 •

edited

Loading

petebacondarwin left a comment

petebacondarwin Nov 21, 2024

petebacondarwin Nov 21, 2024

petebacondarwin Nov 21, 2024

petebacondarwin Nov 21, 2024

petebacondarwin Nov 21, 2024

petebacondarwin Nov 21, 2024

petebacondarwin left a comment

petebacondarwin Nov 21, 2024

petebacondarwin left a comment

petebacondarwin Nov 21, 2024

Turn on wrangler.json(c) support by default #7230

Turn on wrangler.json(c) support by default #7230

Conversation

penalosa commented Nov 11, 2024 • edited Loading

changeset-bot bot commented Nov 11, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Nov 11, 2024 • edited Loading

petebacondarwin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petebacondarwin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petebacondarwin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Turn on `wrangler.json(c)` support by default #7230

Turn on `wrangler.json(c)` support by default #7230

penalosa commented Nov 11, 2024 •

edited

Loading

changeset-bot bot commented Nov 11, 2024 •

edited

Loading

github-actions bot commented Nov 11, 2024 •

edited

Loading