-
Notifications
You must be signed in to change notification settings - Fork 830
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
Allow setting d1 database id in pages dev #3485
Conversation
🦋 Changeset detectedLatest commit: d36687e 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.devprod.cloudflare.dev/workers-sdk/runs/5322823523/npm-package-wrangler-3485 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/3485/npm-package-wrangler-3485 Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/5322823523/npm-package-wrangler-3485 dev path/to/script.js Additional artifacts:npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/5322823523/npm-package-cloudflare-pages-shared-3485 Note that these links will no longer work once the GitHub Actions artifact expires. |
c45d4a3
to
8f36018
Compare
Codecov Report
@@ Coverage Diff @@
## main #3485 +/- ##
==========================================
- Coverage 75.18% 75.08% -0.10%
==========================================
Files 183 183
Lines 11055 11075 +20
Branches 2904 2916 +12
==========================================
+ Hits 8312 8316 +4
- Misses 2743 2759 +16
|
8f36018
to
6349a16
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.
This seems useful until a config file is officially supported, but perhaps would make more sense extending to other bindings too generically, so they share a similar syntax? R2, KV, etc.
Otherwise D1 having this own specific syntax is a little strange.
009a2b4
to
2debdc9
Compare
@@ -32,6 +32,8 @@ const DURABLE_OBJECTS_BINDING_REGEXP = new RegExp( | |||
/^(?<binding>[^=]+)=(?<className>[^@\s]+)(@(?<scriptName>.*)$)?$/ | |||
); | |||
|
|||
const BINDING_REGEXP = new RegExp(/^(?<binding>[^=]+)(?:=(?<ref>[^\s]+))?$/); |
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.
(nit) Is the new RegExp
needed? Seeing as the regex is composed syntactically
828a085
to
db5560f
Compare
d1c625a
to
bfe1d02
Compare
Rebased so fixture tests actually run after #3489 🙃 |
* Allow setting d1 database id in pages dev * Allow KV and R2 to be referenced to too * Fix state directory in fixture test * Add comments describing the binding regexps * Remove duplicate import * More defensively parse bindings
What this PR solves / how to test:
Allows setting a D1 database ID when using
wrangler pages dev
by providing an optional=<ID>
suffix to the argument like--d1 BINDING_NAME=database-id
.This is useful when a Worker and a Pages project are sharing local dev state (
--persist-to
) location.Author has included the following, where applicable:
Reviewer is to perform the following, as applicable: