-
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 experimental_patchConfig #7521
Conversation
🦋 Changeset detectedLatest commit: 6c2ce25 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/12397643964/npm-package-wrangler-7521 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7521/npm-package-wrangler-7521 Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12397643964/npm-package-wrangler-7521 dev path/to/script.js Additional artifacts:wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12397643964/npm-package-cloudflare-workers-bindings-extension-7521 -O ./cloudflare-workers-bindings-extension.0.0.0-vdd3c7f5ec.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-vdd3c7f5ec.vsix npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12397643964/npm-package-create-cloudflare-7521 --no-auto-update npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12397643964/npm-package-cloudflare-kv-asset-handler-7521 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12397643964/npm-package-miniflare-7521 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12397643964/npm-package-cloudflare-pages-shared-7521 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12397643964/npm-package-cloudflare-unenv-preset-7521 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12397643964/npm-package-cloudflare-vitest-pool-workers-7521 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12397643964/npm-package-cloudflare-workers-editor-shared-7521 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12397643964/npm-package-cloudflare-workers-shared-7521 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12397643964/npm-package-cloudflare-workflows-shared-7521 Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
9507a77
to
a4e3f93
Compare
590b62f
to
021ea61
Compare
52fa451
to
04384f1
Compare
9311943
to
a1aa663
Compare
// { | ||
// binding: "KV", | ||
// }, | ||
// 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.
Does this work if you take out the 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.
no :/ I probably need to rethink the 'delete' api, but since no one is asking for it now and all of this is quite separate from the patchConfig flow that is actually used, I was just going to leave it alone for now.
Happy to take the edit with replacement stuff out of this PR as well, and just have the additive patch if you'd rather.
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 still a bit confused about the behaviour. What happens with:
// original
{
kv_namespaces: [
{
binding: "MY_KV"
},
{
binding: "MY_KV2"
}
]
}
// replacement patch
{
kv_namespaces: [
{
binding: "MY_KV"
}
]
}
?
I'd expect the output to be the below, but it sounds like it's something different?
{
kv_namespaces: [
{
binding: "MY_KV"
}
]
}
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.
for future reference:
the edit-with-replacement is going through each terminal? leaf? node and updating that, in order to preserve comments. that's why it needs 'undefined' in order to actually delete something. I suppose the better way to do it would be work out the diff between the patch and original, and only edit those. Will iterate on this later 👍
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.
Couple questions, but otherwise lgtm
a1aa663
to
6c2ce25
Compare
Fixes DEVX 1424