-
Notifications
You must be signed in to change notification settings - Fork 749
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
feat: add options to readConfig() to specify control how we deal with missing environment definitions #7605
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 84cd433 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/12466894588/npm-package-wrangler-7605 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7605/npm-package-wrangler-7605 Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12466894588/npm-package-wrangler-7605 dev path/to/script.js Additional artifacts:wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12466894588/npm-package-cloudflare-workers-bindings-extension-7605 -O ./cloudflare-workers-bindings-extension.0.0.0-v1aabb293d.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-v1aabb293d.vsix npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12466894588/npm-package-create-cloudflare-7605 --no-auto-update npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12466894588/npm-package-cloudflare-kv-asset-handler-7605 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12466894588/npm-package-miniflare-7605 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12466894588/npm-package-cloudflare-pages-shared-7605 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12466894588/npm-package-cloudflare-unenv-preset-7605 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12466894588/npm-package-cloudflare-vitest-pool-workers-7605 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12466894588/npm-package-cloudflare-workers-editor-shared-7605 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12466894588/npm-package-cloudflare-workers-shared-7605 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12466894588/npm-package-cloudflare-workflows-shared-7605 Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
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.
This looks good. Did you mean to also include the change where environment names are not appended if the environment is specified but there is only a top-level environment?
Thanks. I missed that. I fixed it now but it resulted in some renaming of the options to make them make sense. PTAL @jamesopstad |
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.
Thanks Pete. I feel like the following test cases need to be added. They may already pass but I'm not totally confident from looking at the code.
it("should update the worker name if an environment is requested that is defined and allowEnvironmentsWhenNoneAreDefined is true")
it("should update the worker name if an environment name in optionalEnvironments is requested that is defined")
Is the second one the behaviour we agreed on?
Yes, I believe this is what we agreed. If the user has defined the environment that is selected, then it will do its normal processing. I am still wondering about this though. If the user doesn't have a production environment defined and they deploy the worker name will not be suffixed. But as soon as the define an environment it will be suffixed, and they would have to provide a specific Worker name in that environment to get the same name as before. Is this what we want? |
1f111f1
to
f8ebd50
Compare
…ronments In such cases there is no need for the user to define this environment explicitly, even if there are other environments defined. This will be used by the Vite plugin, since `vite dev` command will always set the environment to "development", and we don't want the user to have to define this explicitly if they don't want to.
c14d123
to
84cd433
Compare
Fixes #0000
This will be used by the Vite plugin, since
vite dev
command will always set the environmentto "development", and we don't want the user to have to define this explicitly if they don't want to.
In such cases there is no need for the user to define this environment explicitly,
even if there are other environments defined.