-
Notifications
You must be signed in to change notification settings - Fork 1k
feat(wrangler): support -C, --cwd cli argument
#7994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 5a2e268 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
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/13117239307/npm-package-wrangler-7994You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7994/npm-package-wrangler-7994Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-wrangler-7994 dev path/to/script.jsAdditional artifacts:cloudflare-workers-bindings-extension: wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-workers-bindings-extension-7994 -O ./cloudflare-workers-bindings-extension.0.0.0-v03fe04cf1.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-v03fe04cf1.vsixcreate-cloudflare: npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-create-cloudflare-7994 --no-auto-update@cloudflare/kv-asset-handler: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-kv-asset-handler-7994miniflare: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-miniflare-7994@cloudflare/pages-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-pages-shared-7994@cloudflare/unenv-preset: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-unenv-preset-7994@cloudflare/vite-plugin: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-vite-plugin-7994@cloudflare/vitest-pool-workers: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-vitest-pool-workers-7994@cloudflare/workers-editor-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-workers-editor-shared-7994@cloudflare/workers-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-workers-shared-7994@cloudflare/workflows-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13117239307/npm-package-cloudflare-workflows-shared-7994Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
-C, --cwd cli argument-C, --dir cli argument
-C, --dir cli argument-C, --cwd cli argument
petebacondarwin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and happy with the naming.
|
A minor linting issue to fix https://github.com/cloudflare/workers-sdk/actions/runs/13079905513/job/36575863616?pr=7994#step:5:916 |
|
updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit uneasy with the -C shortening here—I know it's a relatively common convention, but we also have a global flag -c for config, and those seem pretty likely to be mixed up to me. Not a blocker though.
However, this really should have tests.
|
Added basic test in Also made PR for docs: cloudflare/cloudflare-docs#19665 |
2f9e744 to
625faf0
Compare
625faf0 to
5a2e268
Compare
|
@penalosa can you please review this? 🙏🏼 |
| ip: "127.0.0.1", | ||
| inspectorPort: options?.inspectorPort ?? 0, | ||
| v: undefined, | ||
| cwd: undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not related directly to this PR... I wonder if we should actually change StartDevOptions to omit properties like this? Rather than having to pass in undefined?
petebacondarwin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still LGTM
|
I share @penalosa 's concern about |
|
thanks for followup, dear @CarmenPopoviciu. Do you mind to push change to this PR if have time? 🙏🏼 |
yeah, I noticed that being mentioned, so I was a bit torn on what the right thing to do is. Since changing the
totally happy to do that |
|
I've opened a new PR based off of this one, incl. the changes I suggested, so we can get this across the finish line. |
|
Closing this in favour of #8253 |
Fixes #7993 (/cc @petebacondarwin)
This PR adds
-C, --cwdglobal argument to thewranglerCLI to allow changing the current working directory before running any command.This feature is used in most of CLI tools that depend on right working directory but different names. I have chosen
-C, --cwdfollowing with pnpm (-C) + yarn (--cwd), but open to ideas changing it to anything else.This allows better DX of using
wranglercommands from any working directory to another (where project is).--cmdcloudflare-docs#19665