-
Notifications
You must be signed in to change notification settings - Fork 436
feat(astro): Add support for keyless mode #7812
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?
Conversation
🦋 Changeset detectedLatest commit: 0b4fe75 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
a4df941 to
55b6696
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a keyless workflow to the Astro integration: a file-backed keyless service and runtime key resolution utilities with a development-time keyless fallback. Middleware performs per-request keyless resolution and stores keyless data in Astro locals; server/client env merging and get-safe-env prefer injected keyless publishable keys. Integration config/schema accept optional PUBLIC_CLERK_KEYLESS_CLAIM_URL, PUBLIC_CLERK_KEYLESS_API_KEYS_URL, and PUBLIC_CLERK_KEYLESS_DISABLED and make PUBLIC_CLERK_PUBLISHABLE_KEY and CLERK_SECRET_KEY optional. Exports canUseKeyless, adds Playwright tests for keyless flow, and updates Vite and tsup externals. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
📝 WalkthroughWalkthroughThis pull request introduces keyless mode support for the Astro framework. Changes include adding new environment configuration properties for keyless URLs and disabled flags, implementing keyless service initialization with file storage, creating utilities for resolving keys with a keyless fallback mechanism, integrating keyless resolution into the middleware for per-request handling, and adding feature flags to control keyless availability based on environment and filesystem support. A new end-to-end test suite validates the keyless workflow, and the build configuration externals list is updated to include vite. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. No actionable comments were generated in the recent review. 🎉 Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
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: 3
🤖 Fix all issues with AI agents
In `@packages/astro/package.json`:
- Around line 98-102: The lockfile is out of sync with package.json
devDependencies: add vite@^7.1.0; regenerate and commit the updated lockfile
(e.g., run your package manager's install to update lockfile so it includes the
new "vite" entry) and ensure the changed lockfile is included in the PR; verify
the lockfile now contains the vite@^7.1.0 entry corresponding to the
"devDependencies" section in package.json.
In `@packages/astro/src/env.d.ts`:
- Around line 24-25: The InternalEnv interface is missing definitions for
PUBLIC_CLERK_KEYLESS_CLAIM_URL and PUBLIC_CLERK_KEYLESS_API_KEYS_URL which are
referenced by getContextEnvVar in get-safe-env.ts; add both as readonly optional
string properties to the InternalEnv interface (e.g.,
PUBLIC_CLERK_KEYLESS_CLAIM_URL?: string and PUBLIC_CLERK_KEYLESS_API_KEYS_URL?:
string) so keyof InternalEnv matches the keys used and TypeScript compilation
succeeds.
In `@packages/astro/src/server/get-safe-env.ts`:
- Around line 42-43: The calls to getContextEnvVar with
PUBLIC_CLERK_KEYLESS_CLAIM_URL and PUBLIC_CLERK_KEYLESS_API_KEYS_URL fail
TypeScript because those keys are not declared on the InternalEnv type; update
env.d.ts to add these two keys (PUBLIC_CLERK_KEYLESS_CLAIM_URL and
PUBLIC_CLERK_KEYLESS_API_KEYS_URL) to the InternalEnv interface so
getContextEnvVar(...) compiles, ensuring the names and types match other
PUBLIC_* URL entries already defined.
| readonly PUBLIC_CLERK_KEYLESS_DISABLED?: string; | ||
| } |
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.
Missing type definitions for keyless URL environment variables.
The InternalEnv interface is missing PUBLIC_CLERK_KEYLESS_CLAIM_URL and PUBLIC_CLERK_KEYLESS_API_KEYS_URL. These are used in get-safe-env.ts (lines 42-43, 61-62) with getContextEnvVar(), which has a parameter typed as keyof InternalEnv. This will cause TypeScript compilation errors.
🐛 Proposed fix
readonly PUBLIC_CLERK_TELEMETRY_DEBUG?: string;
readonly PUBLIC_CLERK_KEYLESS_DISABLED?: string;
+ readonly PUBLIC_CLERK_KEYLESS_CLAIM_URL?: string;
+ readonly PUBLIC_CLERK_KEYLESS_API_KEYS_URL?: string;
}🤖 Prompt for AI Agents
In `@packages/astro/src/env.d.ts` around lines 24 - 25, The InternalEnv interface
is missing definitions for PUBLIC_CLERK_KEYLESS_CLAIM_URL and
PUBLIC_CLERK_KEYLESS_API_KEYS_URL which are referenced by getContextEnvVar in
get-safe-env.ts; add both as readonly optional string properties to the
InternalEnv interface (e.g., PUBLIC_CLERK_KEYLESS_CLAIM_URL?: string and
PUBLIC_CLERK_KEYLESS_API_KEYS_URL?: string) so keyof InternalEnv matches the
keys used and TypeScript compilation succeeds.
54d9338 to
fc1064b
Compare
- Add keyless service with file storage adapter - Middleware resolves keyless URLs per-request (runtime, not compile-time) - Store keyless data in context.locals - Inject keyless URLs via __CLERK_ASTRO_SAFE_VARS__ script tag - Client reads from params (server-injected) instead of import.meta.env - Add feature flag for keyless mode - Add integration test using shared keyless helpers - Add @clerk/shared as dependency for shared keyless utilities Follows runtime pattern from React Router and TanStack Start: - No browser cache issues - Instant updates when switching modes - Clean separation: Integration for build, Middleware for runtime
64ef8ce to
9546d96
Compare
Refactor packages/astro/src/server/keyless/utils.ts to use the shared resolveKeysWithKeylessFallback utility from @clerk/shared/keyless, matching the pattern used by React Router and TanStack Start. This removes ~70 lines of duplicated code and ensures consistent behavior across all frameworks. Changes: - Import sharedResolveKeysWithKeylessFallback from @clerk/shared/keyless - Re-export KeylessResult type from shared package - Simplify wrapper to create keylessService and call shared utility - Remove duplicated logic (claimed keys flow, cache management, etc.) Benefits: ✅ Keeps codebase DRY ✅ Ensures consistent behavior across frameworks ✅ Makes maintenance easier (single source of truth) ✅ Reduces test surface area
Aligns Astro's keyless implementation with React Router/TanStack Start patterns for consistency and maintainability. Changes: 1. file-storage.ts: - Use static imports without .default destructuring - Pass entire module namespace to createNodeFileStorage - Add error handling with helpful message - Match React Router's implementation pattern 2. keyless/index.ts: - Accept context and options parameters (like React Router) - Add canUseFileSystem() runtime check - Add init promise to prevent race conditions - Add resetKeylessService() for testing - Use createClerkClient directly instead of inline creation - Add proper error handling and logging 3. keyless/utils.ts: - Pass context and options to keyless service - Update function signature to match new pattern 4. feature-flags.ts: - Use getEnvVariable helper instead of direct process.env - Remove hasFileSystemSupport() (handled by keyless service) - Simplify to match React Router's cleaner pattern 5. get-safe-env.ts: - Remove 'as any' type cast (line 59) - Use proper App.Locals type from env.d.ts - Improve type safety throughout 6. clerk-middleware.ts: - Export ClerkAstroMiddlewareOptions type - Pass context and options to resolveKeysWithKeylessFallback Benefits: ✅ Consistent patterns across React Router/TanStack Start/Astro ✅ Better type safety (no 'any' casts) ✅ Cleaner separation of concerns ✅ Race condition protection with init promise ✅ Better error handling and logging ✅ More testable with resetKeylessService() ✅ Follows shared utility patterns (DRY)
Replace manual createClerkClient calls with the existing clerkClient helper, matching React Router's pattern of reusing the framework's client factory. Changes: - Import clerkClient from '../clerk-client' instead of createClerkClient from @clerk/backend - Use clerkClient(context) in createAccountlessApplication and completeOnboarding - Remove manual client configuration (secretKey, publishableKey, apiUrl, userAgent) - Client now properly inherits all configuration from getSafeEnv(context) Benefits: ✅ DRY - reuses existing client configuration logic ✅ Consistent - all API calls use same client setup ✅ Maintainable - client config changes apply everywhere ✅ Matches React Router pattern exactly ✅ Inherits telemetry, sdkMetadata, and all other configs automatically
Fix unused parameters and ensure options are properly passed through the call chain, matching React Router's pattern exactly. Changes: 1. clerk-client.ts: - Update clerkClient signature to accept optional options parameter - Now matches React Router: clerkClient(context, options?) - Allows keyless service to override client configuration 2. keyless/index.ts: - Pass options parameter to clerkClient() calls - Options now used in createAccountlessApplication - Options now used in completeOnboarding - Fixes unused parameter warning 3. clerk-middleware.ts: - Remove unused 'error' variable in catch block - Use empty catch block (matches React Router pattern) 4. keyless/utils.ts: - Fix import sorting (ESLint auto-fix) Benefits: ✅ No unused parameters ✅ Options properly passed through call chain ✅ clerkClient can be overridden with custom config ✅ Matches React Router pattern exactly ✅ All ESLint errors resolved ✅ All TypeScript type checks pass
The options parameter in clerkClient was not providing any value because getSafeEnv(context) already reads all necessary configuration from both process.env and context.locals (which includes keyless values set by middleware). Changes: 1. clerk-client.ts: - Remove optional options parameter from clerkClient signature - Keep createClerkClientWithOptions internal signature for potential future use - Client configuration fully driven by getSafeEnv(context) 2. keyless/index.ts: - Remove ClerkAstroMiddlewareOptions import (no longer needed) - Remove options parameter from keyless() function - Remove options from clerkClient() calls - Simplify function signature 3. keyless/utils.ts: - Remove ClerkAstroMiddlewareOptions import - Remove options parameter from resolveKeysWithKeylessFallback() - Update keyless() call to not pass options 4. clerk-middleware.ts: - Remove options parameter from resolveKeysWithKeylessFallback() call Why this is correct: - getSafeEnv(context) reads from process.env via getContextEnvVar() - getSafeEnv(context) reads keyless values from context.locals - All necessary config (keys, urls, telemetry) is already available - The spread ...options at end of createClerkClient was never overriding anything useful - Simplifies the API and removes unnecessary parameters Benefits: ✅ Simpler API surface ✅ No unused parameters ✅ Clear single source of truth (getSafeEnv) ✅ Less confusing for developers ✅ All type checks pass
Description
Brings keyless mode support to
@clerk/astro, removing the need for API keys during development setup.Changes
@clerk/shared/keyless.env, you'll have to hard reload the browser for keyless to pick it up. I went with runtime middleware instead, similar to react-router and tanstack start keyless implementation.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Configuration
Infrastructure
Tests
Chores