Wrangler and C3: Move the cli folder of C3 into @cloudflare/cli and make Wrangler and C3 depend on it #4189

gabivlj · 2023-10-16T19:59:11Z

What this PR solves:
We want to start using @clack/core and C3-like components in Wrangler, so the logical next step is to make these components available to wrangler and C3.

./helpers/cli.ts in C3 is now index.ts in @cloudflare/cli, didn't include openInBrowser,C3_DEFAULTS and WRANGLER_DEFAULTS.
./helpers/colors in C3 is now colors.ts in @cloudflare/cli.
./helpers/interactive.ts in C3 is now interactive.ts in @cloudflare/cli.

Tests (a basic test)
Changeset (Changeset guidelines)

Reviewer is to perform the following, as applicable:

Checked for inclusion of relevant tests
Checked for inclusion of a relevant changeset
Checked for creation of associated docs updates
Manually pulled down the changes and spot-tested

Note for PR author:

We want to celebrate and highlight awesome PR review! If you think this PR received a particularly high-caliber review, please assign it the label highlight pr review so future reviewers can take inspiration and learn from it.

changeset-bot · 2023-10-16T19:59:15Z

🦋 Changeset detected

Latest commit: bf67717

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

This PR includes changesets to release 3 packages

Name	Type
@cloudflare/cli	Major
create-cloudflare	Patch
wrangler	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 · 2023-10-17T10:21:49Z

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/6625416151/npm-package-wrangler-4189

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

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

Or you can use npx with this latest build directly:

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

Additional artifacts:

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

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

wrangler@3.14.0 includes the following runtime dependencies:

Package	Constraint	Resolved
`miniflare`	3.20231016.0	3.20231016.0
`workerd`	1.20231016.0	1.20231016.0
`workerd --version`	1.20231016.0	2023-10-16

|

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

codecov · 2023-10-17T10:27:47Z

Codecov Report

Merging #4189 (053fb2b) into main (773e2a8) will not change coverage.
Report is 2 commits behind head on main.
The diff coverage is n/a.

❗ Current head 053fb2b differs from pull request most recent head bf67717. Consider uploading reports for the commit bf67717 to get more accurate results

@@           Coverage Diff           @@
##             main    #4189   +/-   ##
=======================================
  Coverage   75.36%   75.36%           
=======================================
  Files         223      223           
  Lines       12259    12259           
  Branches     3171     3171           
=======================================
  Hits         9239     9239           
  Misses       3020     3020

jculvey

Changes LGTM. I've been intending to revisit some of the naming in interactive.ts for a while, which I'd still like to do at some point. I'm curious to hear more about your use case, perhaps we can chat some time.

jculvey · 2023-10-17T16:24:49Z

packages/cli/__tests__/cli.test.ts

@@ -0,0 +1,8 @@
+import { describe, expect, test } from "vitest";


If we want this to run in CI I think we'll have to add a workflow to .github/workflows

packages/cli/.eslintrc.js

gabivlj · 2023-10-17T16:37:31Z

Changes LGTM. I've been intending to revisit some of the naming in interactive.ts for a while, which I'd still like to do at some point. I'm curious to hear more about your use case, perhaps we can chat some time.

Sure! We also want to introduce some changes we did where we added a multiselect component, perhaps we can discuss those naming changes in that future PR and talk more about our interactivity use-cases.

penalosa · 2023-10-18T17:12:12Z

.github/workflows/cli.yml

This file shouldn't be needed—checks should run as part of the monorepo setup

Sure! Just added it because of the suggestion above. Anyway I think that makes sense.

penalosa · 2023-10-18T17:14:55Z

packages/cli/package.json

I don't think we'll want to publish this to npm (at least initially), so this should be set to private

penalosa · 2023-10-18T17:16:43Z

packages/cli/tsconfig.json

+	"compilerOptions": {
+		"target": "ESNext",
+		"module": "CommonJS",
+		"moduleResolution": "node",
+		"allowJs": false,
+		"strict": true,
+		"esModuleInterop": true,
+		"outDir": "dist",
+		"types": ["node"],
+		"resolveJsonModule": true
+	}


How many of these options are necessary given the base config of workers-tsconfig?

Updated to keep it barebones

gabivlj · 2023-10-19T10:31:40Z

Rebased

gabivlj · 2023-10-23T11:47:39Z

Rebased again

…ake Wrangler and C3 depend on it We want to start using @clack/core and C3-like components in Wrangler, so the logical next step is to make these components available to wrangler and C3. `./helpers/cli.ts` in C3 is now index.ts in @cloudflare/cli, didn't include openInBrowser,C3_DEFAULTS and WRANGLER_DEFAULTS. `./helpers/colors` in C3 is now colors.ts in @cloudflare/cli. `./helpers/interactive.ts` in C3 is now interactive.ts in @cloudflare/cli.

gabivlj requested review from a team as code owners October 16, 2023 19:59

gabivlj force-pushed the gv/cli branch from af77e5a to e9a44dd Compare October 16, 2023 20:00

gabivlj force-pushed the gv/cli branch from e9a44dd to 435beb4 Compare October 17, 2023 10:25

gabivlj force-pushed the gv/cli branch from 435beb4 to 34babcb Compare October 17, 2023 10:31

jculvey approved these changes Oct 17, 2023

View reviewed changes

gabivlj force-pushed the gv/cli branch 2 times, most recently from 9d3131c to 65a39a9 Compare October 17, 2023 18:59

penalosa reviewed Oct 18, 2023

View reviewed changes

gabivlj force-pushed the gv/cli branch from 65a39a9 to 60b1356 Compare October 18, 2023 22:07

gabivlj requested a review from penalosa October 18, 2023 22:08

gabivlj force-pushed the gv/cli branch from 60b1356 to dc4d2cf Compare October 19, 2023 10:31

gabivlj force-pushed the gv/cli branch 2 times, most recently from d45e346 to 1a0bc84 Compare October 23, 2023 11:46

penalosa approved these changes Oct 23, 2023

View reviewed changes

gabivlj force-pushed the gv/cli branch 4 times, most recently from d741cd7 to 053fb2b Compare October 23, 2023 14:46

admah added the awaiting Cloudflare response Awaiting response from workers-sdk maintainer team label Oct 23, 2023

gabivlj force-pushed the gv/cli branch from 053fb2b to e929125 Compare October 23, 2023 19:42

gabivlj force-pushed the gv/cli branch from e929125 to bf67717 Compare October 24, 2023 10:12

gabivlj merged commit 0579803 into cloudflare:main Oct 24, 2023
21 of 24 checks passed

workers-devprod mentioned this pull request Oct 24, 2023

Version Packages #4245

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrangler and C3: Move the cli folder of C3 into @cloudflare/cli and make Wrangler and C3 depend on it #4189

Wrangler and C3: Move the cli folder of C3 into @cloudflare/cli and make Wrangler and C3 depend on it #4189

gabivlj commented Oct 16, 2023

changeset-bot bot commented Oct 16, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

codecov bot commented Oct 17, 2023 •

edited

Loading

jculvey left a comment

jculvey Oct 17, 2023

gabivlj commented Oct 17, 2023

penalosa Oct 18, 2023

gabivlj Oct 18, 2023

penalosa Oct 18, 2023

penalosa Oct 18, 2023

gabivlj Oct 18, 2023

gabivlj commented Oct 19, 2023

gabivlj commented Oct 23, 2023

		@@ -0,0 +1,8 @@
		import { describe, expect, test } from "vitest";

Wrangler and C3: Move the cli folder of C3 into @cloudflare/cli and make Wrangler and C3 depend on it #4189

Wrangler and C3: Move the cli folder of C3 into @cloudflare/cli and make Wrangler and C3 depend on it #4189

Conversation

gabivlj commented Oct 16, 2023

changeset-bot bot commented Oct 16, 2023 • edited Loading

🦋 Changeset detected

github-actions bot commented Oct 17, 2023 • edited Loading

codecov bot commented Oct 17, 2023 • edited Loading

Codecov Report

jculvey left a comment

Choose a reason for hiding this comment

jculvey Oct 17, 2023

Choose a reason for hiding this comment

gabivlj commented Oct 17, 2023

penalosa Oct 18, 2023

Choose a reason for hiding this comment

gabivlj Oct 18, 2023

Choose a reason for hiding this comment

penalosa Oct 18, 2023

Choose a reason for hiding this comment

penalosa Oct 18, 2023

Choose a reason for hiding this comment

gabivlj Oct 18, 2023

Choose a reason for hiding this comment

gabivlj commented Oct 19, 2023

gabivlj commented Oct 23, 2023

changeset-bot bot commented Oct 16, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

codecov bot commented Oct 17, 2023 •

edited

Loading