Bucket of core 3 & cicd changes #7389
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nvironmentSetting into Environment resource The DevTools resource only contained a single method (__internal_enableEnvironmentSetting) which has been moved directly to the Environment resource. This simplifies the codebase by removing an unnecessary resource abstraction. Changes: - Remove DevTools resource class and file - Add __internal_enableEnvironmentSetting method to Environment resource - Update EnableOrganizationsPrompt to call environment.__internal_enableEnvironmentSetting directly - Clean up minor formatting inconsistencies in related files
…build esbuild resolves package exports in order and matches the first pattern that fits. The generic "./*" wildcard was placed before the more specific "./internal/clerk-js/*" wildcard, causing esbuild to incorrectly match and fail to resolve nested paths like "@clerk/shared/internal/clerk-js/errors". This issue only manifested when packages were bundled (e.g., by Vite's dependency optimizer) rather than being symlinked from the workspace. It affected integration templates that import @clerk/ui, which in turn imports from @clerk/shared/internal/clerk-js/*. The fix moves "./internal/clerk-js/*" before "./*" so more specific patterns are matched first, following the package exports best practice.
Enhanced loadClerkJsScript and loadClerkUiScript with intelligent error detection and retry logic: - Added hasScriptRequestError() function that uses Performance API to detect failed script loads by checking transferSize, decodedBodySize, responseEnd, and responseStatus - If an existing script has a request error, remove it and retry loading - If waiting for an existing script times out, remove it and allow retry - Added error event listener to bail out early on script load failures instead of waiting for full timeout - Cache scriptUrl to check for errors before attempting to wait for existing script This prevents indefinite hangs when script loads fail and enables automatic recovery from transient network issues.
…URL env vars Nuxt converts camelCase runtime config keys to env var names by inserting underscores before uppercase letters. The keys clerkJSUrl/clerkUiUrl were being converted to NUXT_PUBLIC_CLERK_CLERK_J_S_URL which is not what users expect. Renamed the runtime config keys to jsUrl/uiUrl so they correctly map to NUXT_PUBLIC_CLERK_JS_URL and NUXT_PUBLIC_CLERK_UI_URL, then map them back to clerkJSUrl/clerkUiUrl when passing to the Vue plugin. This allows users to set these values via environment variables using the expected naming convention.
Integration tests were failing because packages published to Verdaccio with the 'integration' dist-tag had transitive dependencies (e.g. @clerk/shared from @clerk/ui) that were resolved from npm instead of the local Verdaccio registry. Changed approach to use snapshot versions that don't exist on npm: - Create unique snapshot versions with `changeset version --snapshot ci` - Publish to 'latest' tag instead of 'integration' tag - Updated linkPackage to return '*' in CI (gets latest snapshot) - Modified snapshot.mjs to always generate changeset for all packages - Updated verdaccio action to check for snapshot versions instead of integration tag - Removed redundant long-running apps from presets that were causing test duplication This ensures all @clerk packages, including transitive dependencies, are installed from the locally published Verdaccio versions.
… tests Integration tests now install packages from Verdaccio with snapshot versions, so they don't need to depend on local builds. Removing these dependencies simplifies the turbo pipeline and allows integration tests to run without waiting for local package builds to complete. Removed dependsOn arrays from all test:integration:* tasks including: - nextjs, nextjs:canary, quickstart - astro, express, ap-flows, generic - localhost, react, remix, tanstack, vue, nuxt, expo Tests will still have access to properly built packages via the snapshot versions published to Verdaccio in the CI pipeline.
Integration tests that create their own applications (using `.clone().commit()`) were failing in CI with "CLERK_FAPI is required" errors. This happened because `setupClerkTestingToken()` needs the CLERK_FAPI environment variable, which is set by calling `clerkSetup()`. Long-running apps automatically call `clerkSetup()` during init, but apps created inline in tests did not. Rather than requiring each test to manually call `clerkSetup()`, moved it into the `Application.dev()` method to run automatically after the server starts. This ensures all applications (both long-running and inline) have proper testing token setup without manual intervention, preventing future test failures and reducing boilerplate. Additionally, added logging of all installed @clerk/* packages (including transitive dependencies) during setup to aid in debugging version mismatches in CI.
Removed Mailsac API integration from emailService as it's no longer needed for integration tests. Tests now use fictional emails with +clerk_test suffix that can be verified using the 424242 OTP without sending actual emails. Added withErrorLogging wrapper function to usersService that logs detailed error information (status, message, errors, clerkTraceId) for all Clerk API calls. This improves debugging when tests fail by providing immediate visibility into API errors without requiring log aggregation. Wrapped all clerkClient calls in usersService with withErrorLogging including: - User operations: createUser, getUser, getUserList, deleteUser - Organization operations: createOrganization, deleteOrganization - API key operations: create, revoke
Simplified billing-related integration tests by removing explicit environment constraints. Tests now run against the default withBilling environment instead of requiring explicit withEnv configuration, reducing test setup complexity. Fixed withBillingJwtV2 environment configuration: - Removed staging API URL override (now uses default production API) - Changed from 'with-billing-staging' to 'with-billing' instance keys - This aligns the test environment with the production billing setup Added CLERK_JS_URL and CLERK_UI_URL to withCustomRoles environment to support local development and CI testing scenarios where custom Clerk script URLs are needed. Also fixed minor typo in test name and removed extraneous blank line.
Simplified integration test configuration and removed redundant setup: - Removed 'integration' dist-tag from linkPackage calls in all presets (astro, express, react, custom-flows) since tests now use snapshot versions from Verdaccio - Renamed CLEANUP to E2E_CLEANUP in constants for better namespacing with other E2E environment variables - Removed manual clerkSetup() calls from custom-flows tests since this is now handled automatically in Application.dev() - Converted machine-auth component test from testAgainstRunningApps pattern to inline app creation, providing more flexibility and reducing long-running app dependencies - Removed MAILSAC_API_KEY from .env.local.sample as mailsac is no longer used - Updated README.md to reflect these changes These changes reduce boilerplate and test complexity while maintaining the same test coverage.
…ization UI Package Updates: - Enhanced billing error handling in CheckoutPage by properly flattening ClerkAPIResponseErrors to extract error codes - Updated InvalidPlanScreen to correctly filter and extract errors from ClerkAPIResponseErrors before checking for invalid_plan_change - Removed unused imports from OrganizationProfile components - Simplified SignUpStart appearance usage - Added type-check script to package.json - Updated tsconfig to exclude __tests__ directories - Enhanced rspack dev config with better source maps and disabled tree shaking for easier debugging - Added __BUILD_DISABLE_RHC__ constant to both rspack and tsdown configs Astro Package Updates: - Refactored createClerkInstance to extract clerk-js and clerk-ui loading into separate functions (getClerkJsEntryChunk, getClerkUiEntryChunk) - Improved code clarity with better comments explaining early returns when scripts are already loaded via middleware - Maintained parallel loading of both scripts for optimal performance Root Package Updates: - Added E2E_DEBUG=1 to billing, generic, and machine integration test scripts for better debugging visibility - Updated billing test to use withBillingJwtV2 instead of withBilling - Fixed duplicate E2E_DEBUG flag in express test script
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Added clerkJSUrl and clerkUiUrl props to integration templates that were missing them. These props are required for CI to use the locally published Clerk packages from Verdaccio instead of the default CDN. Templates updated: - react-router-library - react-router-node - tanstack-react-start
9 tasks
Configure Playwright integration tests to bypass Vercel's deployment protection using the x-vercel-protection-bypass header. This enables automated tests to access protected Vercel deployments without manual intervention. Changes include: - Add extraHTTPHeaders to Playwright config with bypass secret - Whitelist VERCEL_AUTOMATION_BYPASS_SECRET in turbo.json for all integration test tasks - Pass bypass secret from GitHub secrets in CI workflow
nikosdouvlis
force-pushed
the
nikos/fix-cicd-2
branch
from
586f749 to
f3c5005
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes:
Package exports ordering in @clerk/shared
Problem: The wildcard pattern "./" was listed before the specific "./internal/clerk-js/" pattern in package.json exports. esbuild resolves these sequentially and stops at the first match, so it was matching the generic wildcard for nested paths like @clerk/shared/internal/clerk-js/errors and failing. Only showed up in CI because locally we symlink workspace packages, but in CI Vite's optimizer bundles them with esbuild.
Fix: Reordered exports to put specific patterns before generic wildcards.
Verdaccio transitive dependency resolution
Problem: We were publishing packages with the integration dist-tag, but when @clerk/ui@integration imported @clerk/shared@^1.0.0, npm would grab it from the public registry since there was no matching version in Verdaccio under that tag.
Fix: Switched to generating actual snapshot versions that don't exist on npm and publishing to latest instead, so everything has to come from Verdaccio.
showOptionalFields default + layout → options rename
Problem: showOptionalFields was changed to false by default at the same time appearance.layout was renamed to appearance.options. Both changes broke CI for different reasons - tests relied on the old default, and the property rename meant appearance configs weren't being applied.
Fix: Updated all integration templates to use appearance.options with showOptionalFields: true.
Missing clerkJSUrl/clerkUiUrl in templates
Problem: Some templates weren't using local versions of clerk-js/ui in CI.
Fix: Added clerkJSUrl and clerkUiUrl props to: react-router-library, react-router-node, tanstack-react-start.
Nuxt env var naming
Problem: Env vars were getting converted to NUXT_PUBLIC_CLERK_CLERK_J_S_URL instead of NUXT_PUBLIC_CLERK_JS_URL because of how Nuxt converts camelCase.
Fix: Renamed runtime config keys to avoid the double conversion.
DevTools resource missing
Problem: the newly introduced DevTools resource was not being initialized correctly in the components.
Fix: Moved DevTools resource method into Environment resource and deleted the class.
Improvements:
We're now releasing snapshot versions to Verdaccio so e2e tests don't break because of missing changesets.
Integration tests no longer wait for package builds (dependsOn removed from turbo.json), and Verdaccio publishing doesn't rebuild packages - just publishes already-built artifacts from the previous step. This shaves ~2 minutes off the integration test pipeline.
Moved clerkSetup() into Application.dev so tests that create apps inline don't have to call it manually. Was causing "CLERK_FAPI is required" errors.
Added error logging wrapper to usersService that dumps full error details (status, message, API errors, trace ID) when BAPI calls fail.
Dropped Mailsac - only using fictional emails now (+clerk_test with 424242 OTP).
CI now logs all installed @clerk/* package versions after setup, including transitive deps.
Backported smart retry logic to loadClerkJsScript - uses Performance API to detect failed loads and retry instead of waiting for timeout.
TODO
Add a check in integration tests that verifies clerk-js and clerk-ui are always being fetched from localhost. Otherwise we're not actually testing local changes.
Build is fast (rspack, rolldown, esbuild) but type checking is slow. Separate type checking from build commands so we can skip it during integration tests and only verify types in the build-packages job. Faster dev cycles.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change