chore(wrangler): Configuration keys clean-up #5123
Conversation
|
|
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/8775208500/npm-package-wrangler-5123You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/5123/npm-package-wrangler-5123Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8775208500/npm-package-wrangler-5123 dev path/to/script.jsAdditional artifacts:npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8775208500/npm-package-create-cloudflare-5123 --no-auto-updatenpm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8775208500/npm-package-cloudflare-kv-asset-handler-5123npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8775208500/npm-package-miniflare-5123npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8775208500/npm-package-cloudflare-pages-shared-5123npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8775208500/npm-package-cloudflare-vitest-pool-workers-5123Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5123 +/- ##
==========================================
- Coverage 72.44% 70.41% -2.04%
==========================================
Files 331 298 -33
Lines 17298 15519 -1779
Branches 4422 3990 -432
==========================================
- Hits 12532 10927 -1605
+ Misses 4766 4592 -174 |
| * | ||
| * // Carmen according to our tests the default is undefined |
There was a problem hiding this comment.
The default isn't quite undefined: it's a bit weird. It's documented https://developers.cloudflare.com/workers/configuration/routing/routes/#routes-with-workersdev, but essentially the default is true unless you have route or routes defined, in which case the default is false.
| @@ -423,13 +417,13 @@ interface EnvironmentNonInheritable { | |||
| * NOTE: This field is not automatically inherited from the top level environment, | |||
| * and so must be specified in every named environment. | |||
| * | |||
| * @default `{}` | |||
| * @default `{consumers:[],producers:[]}` | |||
There was a problem hiding this comment.
as per our tests, this seems to be the default
| * @default `[]` | ||
| * @nonInheritable | ||
| */ | ||
| dispatch_namespaces: { |
There was a problem hiding this comment.
can we pls confirm this is indeed nonInheritable? The code comment seems to point to that, but the key was listed in the inheritable list
There was a problem hiding this comment.
Based on this line...
...it seems like this is indeed non inheritable.
There was a problem hiding this comment.
In which case it is in the incorrect interface?
There was a problem hiding this comment.
indeed. will fix that!
As part of an ongoing "configuration keys in wrangler" audit, this commit cleans up some of the configuration related comments, annotations, aso, in an attempt to keep things consistent and updated across the board.
petebacondarwin
force-pushed
the
carmen/config-keys-cleanup
branch
3 times, most recently
from
cd0880d
to
3c26de4
Compare
As part of an ongoing "configuration keys in wrangler" audit, this commit cleans up some of the configuration related comments, annotations, aso, in an attempt to keep things consistent and updated across the board.
Fixes # [insert GH or internal issue number(s)].
What this PR solves / how to test:
Author has addressed the following:
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 reviewso future reviewers can take inspiration and learn from it.