-
Notifications
You must be signed in to change notification settings - Fork 82
feat(cloudflare): make Hyperdrive origin optional for local development #1146
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
base: main
Are you sure you want to change the base?
feat(cloudflare): make Hyperdrive origin optional for local development #1146
Conversation
…1093) * test(cloudflare): comprehensive dead letter queue tests Adds comprehensive test coverage for the dead letter queue feature in QueueConsumer, which was fixed in PR alchemy-run#1092. Tests cover: - DLQ with string reference (queue name) - DLQ with Queue object reference - Updating consumer to add DLQ - Worker eventSources with DLQ settings Also includes the missing fix for build-worker-options.ts to properly handle deadLetterQueue in Miniflare local development, and updates queue-consumer.ts to preserve DLQ settings from props when the API doesn't return them in the response. Related: alchemy-run#1092 * test(cloudflare): comprehensive dead letter queue tests Adds comprehensive test coverage for the dead letter queue feature in QueueConsumer, which was fixed in PR alchemy-run#1092. Tests cover: - DLQ with string reference (queue name) - DLQ with Queue object reference - Updating consumer to add DLQ - Worker eventSources with DLQ settings Also includes the missing fix for build-worker-options.ts to properly handle deadLetterQueue in Miniflare local development, and updates queue-consumer.ts to preserve DLQ settings from props when the API doesn't return them in the response. Related: alchemy-run#1092 * Improved the DLQ test and fixed the typings for being root not in settings * Removed DLQ response, since Cloudflare missed that DX in the API we will callback to the props instead for something to be there for the user --------- Co-authored-by: Sam Goodwin <[email protected]>
Improves local-first development workflow by making the `origin` property optional when only `dev.origin` is provided. This matches the wrangler.jsonc pattern where developers can use a local connection string without needing production database credentials. Changes: - Made `origin` optional in `HyperdriveProps` - Added runtime validation that throws a clear error if deploying to production without `origin` - In local mode, `origin` falls back to `dev.origin` if not provided - Updated documentation with "Local Development Only" example - All existing tests pass (fully backwards compatible) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes make the origin property optional in HyperdriveProps, add conditional logic to enforce production origin requirements while supporting local development with dev.origin fallback, update documentation with local development guidance, introduce a new test for the production validation error, and create a new example project demonstrating dev-only Hyperdrive setup. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Hyperdrive
participant Config
participant Error
Caller->>Hyperdrive: Create with optional origin
alt Production Mode (not local scope)
Hyperdrive->>Config: Check if origin provided
alt Origin Missing
Config->>Error: Throw "requires 'origin' for production deployment"
Error-->>Caller: Error
else Origin Provided
Config->>Hyperdrive: Use production origin
Hyperdrive-->>Caller: Success
end
else Local Mode
Hyperdrive->>Config: origin not provided
Config->>Hyperdrive: Use dev.origin fallback
Hyperdrive-->>Caller: Success with dev connection
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes involve conditional logic modifications in the core Hyperdrive implementation with production/local branching, new test coverage, documentation updates, and a complete new example project. The logic is moderately dense but localized to origin handling, making review straightforward for each cohort despite the file spread. Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (4 warnings)
✅ Passed checks (2 passed)
Comment |
@coderabbitai can you review this PR please? |
@Mkassabov: I'll review this PR for you. Let me analyze the changes to ensure the implementation is correct and the local development workflow improvement works as intended. ✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
alchemy/test/cloudflare/hyperdrive.test.ts (1)
171-188
: Simplify test: NeonProject creation appears unnecessary.The test creates a NeonProject but then uses its connection parameters only for
dev.origin
. Since the test validates that a productionorigin
is required (and should fail without it), creating an actual Neon database seems wasteful. A simpler local connection string would suffice fordev.origin
.Consider simplifying the test to avoid unnecessary resource creation:
test("hyperdrive with no production origin throws", async (scope) => { try { - const project = await NeonProject(`${testId}-dev-only`, { - name: `Hyperdrive Test Dev Only ${BRANCH_PREFIX}`, - }); - await expect( Hyperdrive(`${testId}-dev`, { name: `test-hyperdrive-dev-${BRANCH_PREFIX}`, dev: { - origin: project.connection_uris[0].connection_parameters, + origin: "postgres://postgres:postgres@localhost:5432/postgres", }, }), ).rejects.toThrowError(/requires 'origin' for production deployment/); } finally { await destroy(scope); } });examples/cloudflare-dev-only-hyperdrive/alchemy.run.ts (1)
16-21
: Consider clarifying the example's purpose.The example name "cloudflare-dev-only-hyperdrive" suggests a setup that only works locally (like the docs example with
localhost:5432
), but this example creates production infrastructure (PrismaPostgres) and conditionally omits the productionorigin
in local mode.While the code is correct, this pattern is more of a "progressive configuration" workflow rather than a true "dev-only" setup. Consider either:
- Renaming the example to better reflect what it demonstrates (e.g.,
cloudflare-hyperdrive-progressive-config
)- Changing the example to use a hardcoded localhost connection string like the docs show
- Adding a comment explaining this pattern is for teams using cloud databases in local development
alchemy/src/cloudflare/hyperdrive.ts (1)
317-340
: Validate that at least one origin is provided.The validation logic correctly handles the production case (line 321), but in local mode, if neither
origin
nordev.origin
is provided, the code will fail at runtime when trying to normalizeundefined
(line 332). This would result in an unclear error message.Consider adding validation to ensure at least one is provided:
// In local mode, origin can be omitted if dev.origin is provided const devOrigin = props.dev?.origin; const productionOrigin = props.origin; if (!Scope.current.local && !productionOrigin) { throw new Error( `Hyperdrive "${id}" requires 'origin' for production deployment. ` + `Add the production database connection to enable deployment.\n\n` + `For local development only, you can omit 'origin' and only provide 'dev.origin'.`, ); } + // Ensure at least one origin is provided + if (!productionOrigin && !devOrigin) { + throw new Error( + `Hyperdrive "${id}" requires either 'origin' or 'dev.origin'. ` + + `Provide at least one database connection configuration.`, + ); + } + // Use dev.origin as fallback if origin is not provided (local mode only) const origin = productionOrigin ? normalizeHyperdriveOrigin(productionOrigin) : normalizeHyperdriveOrigin(devOrigin!);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lock
is excluded by!**/*.lock
📒 Files selected for processing (11)
alchemy-web/src/content/docs/providers/cloudflare/hyperdrive.md
(1 hunks)alchemy/src/cloudflare/hyperdrive.ts
(2 hunks)alchemy/test/cloudflare/hyperdrive.test.ts
(1 hunks)examples/cloudflare-dev-only-hyperdrive/README.md
(1 hunks)examples/cloudflare-dev-only-hyperdrive/alchemy.run.ts
(1 hunks)examples/cloudflare-dev-only-hyperdrive/package.json
(1 hunks)examples/cloudflare-dev-only-hyperdrive/src/worker.ts
(1 hunks)examples/cloudflare-dev-only-hyperdrive/tsconfig.json
(1 hunks)examples/cloudflare-tanstack-start/src/routeTree.gen.ts
(2 hunks)package.json
(1 hunks)tests/smoke-test-flatten-website/package.json
(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-10-19T21:11:37.829Z
Learnt from: CR
PR: alchemy-run/alchemy#0
File: alchemy/templates/rwsdk/.cursor/rules/alchemy_cloudflare.mdc:0-0
Timestamp: 2025-10-19T21:11:37.829Z
Learning: Applies to alchemy/templates/rwsdk/**/worker.ts : Type worker implementations with env: typeof worker.Env in the default export’s fetch handler
Applied to files:
examples/cloudflare-dev-only-hyperdrive/src/worker.ts
📚 Learning: 2025-10-19T21:13:28.624Z
Learnt from: CR
PR: alchemy-run/alchemy#0
File: alchemy/templates/rwsdk/.cursor/rules/rwsdk_rwsdk-request-response.mdc:0-0
Timestamp: 2025-10-19T21:13:28.624Z
Learning: Applies to alchemy/templates/rwsdk/src/worker.tsx : Import co-located routes into defineApp using prefix()
Applied to files:
examples/cloudflare-tanstack-start/src/routeTree.gen.ts
📚 Learning: 2025-10-19T21:10:23.073Z
Learnt from: CR
PR: alchemy-run/alchemy#0
File: .cursor/rules/cloudflare.mdc:0-0
Timestamp: 2025-10-19T21:10:23.073Z
Learning: Applies to alchemy/src/cloudflare/!{bindings,bound,worker}.ts : Create a new resource file at alchemy/src/cloudflare/{resource}.ts for the added resource
Applied to files:
examples/cloudflare-dev-only-hyperdrive/tsconfig.json
examples/cloudflare-dev-only-hyperdrive/alchemy.run.ts
📚 Learning: 2025-10-19T23:34:43.575Z
Learnt from: CR
PR: alchemy-run/alchemy#0
File: AGENTS.md:0-0
Timestamp: 2025-10-19T23:34:43.575Z
Learning: Applies to examples/*/tsconfig.json : Each example project’s tsconfig.json must extend ../../tsconfig.base.json
Applied to files:
examples/cloudflare-dev-only-hyperdrive/tsconfig.json
📚 Learning: 2025-10-19T23:34:43.575Z
Learnt from: CR
PR: alchemy-run/alchemy#0
File: AGENTS.md:0-0
Timestamp: 2025-10-19T23:34:43.575Z
Learning: Applies to examples/*/alchemy.run.ts : Each example must include an alchemy.run.ts used to deploy and tear down resources
Applied to files:
examples/cloudflare-dev-only-hyperdrive/tsconfig.json
examples/cloudflare-dev-only-hyperdrive/alchemy.run.ts
📚 Learning: 2025-10-19T21:10:23.073Z
Learnt from: CR
PR: alchemy-run/alchemy#0
File: .cursor/rules/cloudflare.mdc:0-0
Timestamp: 2025-10-19T21:10:23.073Z
Learning: Applies to alchemy/src/cloudflare/worker.ts : Update alchemy/src/cloudflare/worker.ts to map the new binding to the Cloudflare metadata API
Applied to files:
examples/cloudflare-dev-only-hyperdrive/alchemy.run.ts
🔇 Additional comments (7)
tests/smoke-test-flatten-website/package.json (1)
20-20
: Verify the version pinning strategy for this smoke test.The alchemy dependency was changed from
"workspace:*"
to"0.62.2"
, which pins to a specific version rather than using the workspace protocol. In a monorepo, smoke tests typically use"workspace:*"
to test against the current local version of the package.Is this version pinning intentional? If so, please clarify the reasoning. Otherwise, consider reverting to
"workspace:*"
to ensure the smoke test runs against the local development version.examples/cloudflare-dev-only-hyperdrive/package.json (1)
1-18
: LGTM!The package structure follows monorepo conventions correctly, using
workspace:*
for the alchemy dependency andcatalog:
for typescript.examples/cloudflare-tanstack-start/src/routeTree.gen.ts (1)
1-177
: Auto-generated file - no review needed.This file is explicitly marked as auto-generated by TanStack Router (lines 7-9) and contains only formatting changes (quote style). Auto-generated files should not be manually reviewed.
examples/cloudflare-dev-only-hyperdrive/tsconfig.json (1)
1-8
: LGTM!The TypeScript configuration correctly extends the base config and follows the established pattern for example projects.
Based on learnings.
alchemy-web/src/content/docs/providers/cloudflare/hyperdrive.md (1)
41-56
: Clear documentation of the new local-first development pattern.The new section effectively explains how to use Hyperdrive for local development without production credentials. The example is straightforward, and the caution note appropriately warns developers about production requirements.
alchemy/src/cloudflare/hyperdrive.ts (2)
154-158
: Well-documented interface change.The updated comments clearly explain that
origin
is optional in local mode but required for production deployments. This aligns with the implementation logic.
313-346
: Backward compatible implementation.The changes maintain backward compatibility - existing code that provides
origin
continues to work as before. The new optional behavior enables local-first development workflows without breaking existing deployments.
Improves local-first development workflow by making the
origin
property optional when onlydev.origin
is provided. This matches thewrangler.jsonc pattern where developers can use a local connection string without needing production database credentials.
Changes
origin
optional inHyperdriveProps
interfaceorigin
origin
falls back todev.origin
if not providedExample Usage
Before (required production credentials even for local dev):