-
Notifications
You must be signed in to change notification settings - Fork 757
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
fix: only send durable object migrations when required #728
Conversation
🦋 Changeset detectedLatest commit: 47ed919 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/2071558805/npm-package-wrangler-728 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/728/npm-package-wrangler-728 Or you can use npx https://prerelease-registry.developers.workers.dev/runs/2071558805/npm-package-wrangler-728 dev path/to/script.js |
2228cd9
to
d6a156c
Compare
).length > 0 && | ||
config.migrations.length === 0 | ||
) { | ||
console.warn( |
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 is a warning and not an error because folks may have purposely deleted the [migrations]
section from their wrangler.toml
altogether. That's not great, but it's not something we can prevent.
I'm going to remove the exports validation from this PR and send it in the next one |
d6a156c
to
adf7c66
Compare
Ok done, this is ready for review. |
adf7c66
to
4489c7b
Compare
We had a bug where even if you'd published a script with migrations, we would still send a blank set of migrations on the next round. The api doesn't accept this, so the fix is to not do so. I also expanded test coverage for migrations. Fixes #705
4489c7b
to
47ed919
Compare
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.
Ship it!
We had a bug where even if you'd published a script with migrations, we would still send a blank set of migrations on the next round. The api doesn't accept this, so the fix is to not do so. I also expanded test coverage for migrations.
Fixes #705
This is the first in a set of a few PRs I'll send for durable objects. Next, more validations based on exports.