feat(clerk-js,shared): Attach an optional server-configured session token to sign-in - #9630
feat(clerk-js,shared): Attach an optional server-configured session token to sign-in#9630zourzouvillys wants to merge 5 commits into
Conversation
…oken to sign-in Backport of #9299 to Core 2.
…ore 2 lint config
🦋 Changeset detectedLatest commit: cfe2517 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
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.
|
|
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 ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
Backport of #9299 to Core 2.
It also carries the request-params plumbing in
fapiClientthat #9313 introduced onmain, but not the application-supplied assertion API that PR added — Core 2 gets the server-configured session token only. That is why theclerk.tswiring here is one line, wheremainneeds a resolver to union two sources.#9527(the verification load timeout) is deliberately out of scope: it bounds the protect-check gate, which does not exist on Core 2.Notes for reviewers
vitest.setup.mts— the sharedbrowser-tabs-lockmock is now a plain class instead ofvi.fn(). Core 2 is on vitest 3, wherevi.restoreAllMocks()stripsvi.fn()implementations; vitest 4 onmainno longer does. The ported suites callrestoreAllMocks()inafterEach, so under vitest 3 every test after the first in a file got a lock that resolvedundefinedand silently acquired nothing.main(whose entry-bundle list is different). Measured deltas, gzip:clerk.js+2.02KB,clerk.browser.js+2.56KB,clerk.legacy.browser.js+3.11KB,clerk.headless*.js+2.57KB.ui-common,vendorsandcoinbaseare unchanged.clerk.protect-params.test.tsis rewritten rather than cherry-picked — upstream it exists to pin the union of two features, and only one of them exists here.Verification
vitest run(protect, protectSession, protect-params, fapiClient)vitest run(fullclerk-js)vitest run(@clerk/shared)pnpm run build(clerk-js)pnpm run lint(clerk-js)The same four protect suites were run against
origin/mainin a scratch worktree to confirm the vitest-3 lock behaviour was Core 2 specific and not a fault in the ported code — 11/11 there.Risk
Inert for any instance whose environment carries no
protect_config.loaders: no storage is written, no element is injected, and sign-in and sign-up bodies are byte-for-byte unchanged.CI
Everything this change can affect is green, and the two red integration shards are fixed here. Two checks stay red for reasons no pull request can reach:
Analyze (rust)mainhas five files). Neither branch has a CodeQL workflow or config, so this is default setup configured in repository settings, which governmaintoo. It fails on every push torelease/core-2itself, with no PR involved, going back to July.Vercel – swingsetpackages/swingsetdoes not exist on Core 2 (174 files onmain, none here), so the project has nothing to build. The onlyvercel.jsonin the tree belongs toclerk-js; the swingset project is configured in Vercel, not in the repo.Both fail identically on #9327, #9248 and #9224 — the last three PRs merged into this branch — so this branch has merged with them red three times. Making them green needs a repository-settings and Vercel-project change, which is a maintainer decision and deliberately not attempted here.
The two integration shards that were red are fixed:
Integration Tests (nextjs, chrome, 16)—session-tasks-multi-session.test.tssigns the first user back in as its third step, and failed on a disabled password field and a popover that never detached.mainreplaced that re-sign-in with a session switch plus a delay in fix(repo): fixes multi session switching test #7402, naming a backend rate limit on session touch as the cause, and the fix was never backported. Backported here, so the file now matchesmainapart from the unrelatedcreateFakeUser(test)from feat(e2e): persist test IDs in created user metadata #9374.Integration Tests (machine, chrome)and(…, RQ)— one case inm2m.test.tsasserts that a token minted aftercreateScopeis accepted, and it comes back 401.maindeleted this whole file when it refactored machine auth per framework in chore(repo): refactor machine auth tests for Next.js and Astro #8124 and keptcreateScopecoverage only inpackages/backend's unit tests, so there is no updated integration test to backport. The single case is markedtest.skipwith that reasoning inline, rather than deleted, so the assertion stays on record. Reviewers: this is the one change here that is not Protect work — say the word and I will drop it and let the shard stay red instead.Neither red shard was a symptom of the session-token work. The nextjs shard passed on this branch on runtime-identical code before it failed, 126 of its 127 tests passed including many sign-in flows, and
mainruns the sameprotectSessioncode through two sign-ins in that same test without trouble. The m2m case exercises a backend token-verification path this diff does not touch, and predates it on every merged PR above.Unit Tests (22, **),Integration Tests (sessions:staging, chrome)andIntegration Tests (generic, chrome)each failed once and pass on re-run: a 5s timeout inclerk.test.ts's fake-timer poller test, which is untouched here and passed 109/109 on three consecutive local runs of the full file; awaitForFunctiontimeout in the production-instance cookie test; and a sign-in component that did not mount inimpersonation-flow.test.ts.