chore: defineCommand refactor for `wrangler types` and `tail` #7220

emily-shen · 2024-11-11T13:16:46Z

moving another couple of commands to defineCommand
related #6655

Tests
- TODO (before merge)
- Tests included
- Tests not necessary because: no behaviour change, just needs to pass existing
E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
- I don't know
- Required
- Not required because: no behaviour change, just needs to pass existing
Public documentation
- TODO (before merge)
- Cloudflare docs PR(s):
- Documentation not necessary because: internal refactor

changeset-bot · 2024-11-11T13:16:50Z

⚠️ No Changeset found

Latest commit: 1338784

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

github-actions · 2024-11-11T13:21:56Z

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/11854761777/npm-package-wrangler-7220

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

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

Or you can use npx with this latest build directly:

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

Additional artifacts:

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

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

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

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

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

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

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

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

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.0
`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/tail/index.ts

edmundhung · 2024-11-13T15:12:31Z

packages/wrangler/src/type-generation/index.ts

+		const validInterfaceRegex = /^[a-zA-Z][a-zA-Z0-9_]*$/;

-	if (!validInterfaceRegex.test(envInterface)) {
-		throw new CommandLineArgsError(
-			`The provided env-interface value ("${envInterface}") does not satisfy the validation regex: ${validInterfaceRegex}`
-		);
-	}
+		if (!validInterfaceRegex.test(envInterface)) {
+			throw new CommandLineArgsError(
+				`The provided env-interface value ("${envInterface}") does not satisfy the validation regex: ${validInterfaceRegex}`
+			);
+		}

-	if (!outputPath.endsWith(".d.ts")) {
-		throw new CommandLineArgsError(
-			`The provided path value ("${outputPath}") does not point to a declaration file (please use the 'd.ts' extension)`
-		);
-	}
+		if (!outputPath.endsWith(".d.ts")) {
+			throw new CommandLineArgsError(
+				`The provided path value ("${outputPath}") does not point to a declaration file (please use the 'd.ts' extension)`
+			);
+		}


The difference you see on the tests seems to be caused by us calling readConfig before the handler. Move these logic to validateArgs might fix it

ugh it isn't failing locally, that's super weird - but good point
edit: nope, the tests are failing for tail and this is for types (sorry again for having the two in one PR 😅 ...)

oh sorry do you mean the tests I modify? Yeah those have changed because readConfig is running first. Don't think moving these into validateArgs would fix it, but arguably makes sense to do anyway 👍

Could have moved the config file check into validateArgs, but I thought it would be better to have readConfig handle the case where the specified config file doesn't exist since that's a general error (hence getting a ParseError now), and the type gen handler to handle the case where there is no config file at all, since that's a wrangler types specific error.

penalosa · 2024-11-14T16:00:06Z

packages/wrangler/src/__tests__/type-generation.test.ts

+		await expect(
+			runWrangler("types -c hello.toml")
+		).rejects.toThrowErrorMatchingInlineSnapshot(
+			`[ParseError: Could not read file: hello.toml]`


This seems like a slightly worse error message—is there any way we can improve it?

think its okay as it does also output ENOENT: no such file or directory... but that didn't show up in the snapshot for some reason 🤔

emily-shen requested a review from a team as a code owner November 11, 2024 13:16

emily-shen force-pushed the emily/type-gen-cmd branch from e05e304 to f087b10 Compare November 13, 2024 13:55

emily-shen added the no-changeset-required label Nov 13, 2024

edmundhung reviewed Nov 13, 2024

View reviewed changes

emily-shen force-pushed the emily/type-gen-cmd branch from 41a2720 to 8be89ec Compare November 14, 2024 11:27

emily-shen requested review from a team as code owners November 14, 2024 11:27

emily-shen force-pushed the emily/type-gen-cmd branch from 07d09c6 to 2669795 Compare November 14, 2024 12:54

penalosa reviewed Nov 14, 2024

View reviewed changes

emily-shen added 10 commits November 15, 2024 10:39

use defineCommand for wrangler types

81f44e4

wrangler tail

fc7c5e8

fix format default for wrangler tail

81fdd40

fix test?

b41fd58

reorder imports

1ade94c

maybe fix tests this time?

3678e2e

please actually fix this time

f56b30c

definitely fixed this time

cad207d

please

2dba806

lint

1338784

emily-shen force-pushed the emily/type-gen-cmd branch from 50590c5 to 1338784 Compare November 15, 2024 10:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: defineCommand refactor for `wrangler types` and `tail` #7220

chore: defineCommand refactor for `wrangler types` and `tail` #7220

emily-shen commented Nov 11, 2024 •

edited

Loading

changeset-bot bot commented Nov 11, 2024 •

edited

Loading

github-actions bot commented Nov 11, 2024 •

edited

Loading

edmundhung Nov 13, 2024

emily-shen Nov 13, 2024 •

edited

Loading

emily-shen Nov 13, 2024

penalosa Nov 14, 2024

emily-shen Nov 14, 2024 •

edited

Loading

chore: defineCommand refactor for wrangler types and tail #7220

Are you sure you want to change the base?

chore: defineCommand refactor for wrangler types and tail #7220

Conversation

emily-shen commented Nov 11, 2024 • edited Loading

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

⚠️ No Changeset found

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

edmundhung Nov 13, 2024

Choose a reason for hiding this comment

emily-shen Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

emily-shen Nov 13, 2024

Choose a reason for hiding this comment

penalosa Nov 14, 2024

Choose a reason for hiding this comment

emily-shen Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

chore: defineCommand refactor for `wrangler types` and `tail` #7220

chore: defineCommand refactor for `wrangler types` and `tail` #7220

emily-shen commented Nov 11, 2024 •

edited

Loading

changeset-bot bot commented Nov 11, 2024 •

edited

Loading

github-actions bot commented Nov 11, 2024 •

edited

Loading

emily-shen Nov 13, 2024 •

edited

Loading

emily-shen Nov 14, 2024 •

edited

Loading