fix: add better error for user binding called ASSETS in pages projects #7141
Conversation
🦋 Changeset detectedLatest commit: f405bf5 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/11614725251/npm-package-wrangler-7141You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7141/npm-package-wrangler-7141Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11614725251/npm-package-wrangler-7141 dev path/to/script.jsAdditional artifacts:npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11614725251/npm-package-create-cloudflare-7141 --no-auto-updatenpm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11614725251/npm-package-cloudflare-kv-asset-handler-7141npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11614725251/npm-package-miniflare-7141npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11614725251/npm-package-cloudflare-pages-shared-7141npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11614725251/npm-package-cloudflare-vitest-pool-workers-7141npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11614725251/npm-package-cloudflare-workers-editor-shared-7141npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11614725251/npm-package-cloudflare-workers-shared-7141npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11614725251/npm-package-cloudflare-workflows-shared-7141Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
| @@ -2773,6 +2773,12 @@ const validateBindingsHaveUniqueNames = ( | |||
| bindingsGroupedByName[bindingName] = []; | |||
| } | |||
|
|
|||
| if (bindingName === "ASSETS" && isPagesConfig(config)) { | |||
There was a problem hiding this comment.
I decided it was okay to just shove the check in here as a one-off, because I don't think we have any other reserved binding names (correct me if i'm wrong though) and we should absolutely not introduce any new binding names that could conflict like this either.
|
a binding named ASSETS is allowed in workers projects though, and there are plenty of existing tests using that so that case should already be covered |
| @@ -2773,6 +2773,12 @@ const validateBindingsHaveUniqueNames = ( | |||
| bindingsGroupedByName[bindingName] = []; | |||
| } | |||
|
|
|||
| if (bindingName.toUpperCase() === "ASSETS" && isPagesConfig(config)) { | |||
| diagnostics.errors.push( | |||
| `The name 'ASSETS' is reserved in Pages projects. Please use a different name for your ${bindingType} binding.` | |||
There was a problem hiding this comment.
Nit: Is binding name case sensitive? Might be better to show the original name user provided.
| `The name 'ASSETS' is reserved in Pages projects. Please use a different name for your ${bindingType} binding.` | |
| `The name '${bindingName}' is reserved in Pages projects. Please use a different name for your ${bindingType} binding.` |
There was a problem hiding this comment.
turns out it is case sensitive, so you totally can deploy with 'assets' (lowercase) no issues
Fixes #6266
The binding name "ASSETS" is used internally by pages projects. If a user had a binding with this name (totally sensible binding name for an r2 bucket for example) it would fail cryptically (see linked issue).