fix: normalise account_id = '' to account_id: undefined
#1365
Conversation
In older templates, (i.e made for wrangler 1.x), `account_id =''` is considered as a valid input, but then ignored. With wrangler 2, when running wrangler dev, we log an error, but it fixes itself after we get an account id. Much like #1329, the fix here is to normalise that value when we see it, and replace it with `undefined` while logging a warning. This fix also tweaks the messaging for a blank route value to suggest some user action.
🦋 Changeset detectedLatest commit: 78e208e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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.developers.workers.dev/runs/2577572179/npm-package-wrangler-1365You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/1365/npm-package-wrangler-1365Or you can use npx https://prerelease-registry.developers.workers.dev/runs/2577572179/npm-package-wrangler-1365 dev path/to/script.js |
There was a problem hiding this comment.
We should have further discussions internally pertaining to null vs. undefined vs values like[ ]
Cc: @petebacondarwin
This isn't blocking and my question for messaging is also non-blocking
| diagnostics: Diagnostics, | ||
| rawEnv: RawEnvironment |
There was a problem hiding this comment.
I see that it's common we are passing diagnostics first.
| ): RawEnvironment { | ||
| if (rawEnv["route"] === "") { | ||
| diagnostics.warnings.push( | ||
| `Route assignment ${JSON.stringify(rawEnv["route"])} will be ignored.` | ||
| `The "route" field in your configuration is an empty string and will be ignored.\n` + | ||
| `Please remove the "route" field from your configuration.` |
There was a problem hiding this comment.
Allowing the empty string, then asking them to remove the field seems ambivalent.
There was a problem hiding this comment.
It's more actionable than just saying it'll be ignored, this gives them an action to take to remove the warning.
It'll happen when they use an older project or template. We'll have a go at cleaning up the templates when we work on generate, but we should also give actionable advice.
Disable whitespace for this review!
In older templates, (i.e made for wrangler 1.x),
account_id =''is considered as a valid input, but then ignored. With wrangler 2, when running wrangler dev, we log an error, but it fixes itself after we get an account id. Much like #1329, the fix here is to normalise that value when we see it, and replace it withundefinedwhile logging a warning.This fix also tweaks the messaging for a blank route value to suggest some user action.