From 3acbceb6d8f0946a5567bae221f43f1f8869e56c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Tue, 3 Nov 2026 13:36:15 +0100 Subject: [PATCH 01/26] Implement basic routing with virtual mode --- packages/mosaic/src/mosaic-provider.tsx | 11 +- .../src/routing/__tests__/router.test.ts | 59 +++++++ .../src/routing/__tests__/routes.test.ts | 119 +++++++++++++ .../src/routing/__tests__/use-router.test.tsx | 160 ++++++++++++++++++ .../__tests__/virtual-location.test.tsx | 124 ++++++++++++++ packages/mosaic/src/routing/location.ts | 14 ++ packages/mosaic/src/routing/router.ts | 44 +++++ packages/mosaic/src/routing/routes.test-d.ts | 38 +++++ packages/mosaic/src/routing/routes.ts | 160 ++++++++++++++++++ packages/mosaic/src/routing/routing-scope.tsx | 31 ++++ .../mosaic/src/routing/use-mosaic-location.ts | 20 +++ packages/mosaic/src/routing/use-router.ts | 109 ++++++++++++ .../mosaic/src/routing/virtual-location.ts | 57 +++++++ packages/mosaic/src/routing/virtual-url.tsx | 33 ++++ 14 files changed, 975 insertions(+), 4 deletions(-) create mode 100644 packages/mosaic/src/routing/__tests__/router.test.ts create mode 100644 packages/mosaic/src/routing/__tests__/routes.test.ts create mode 100644 packages/mosaic/src/routing/__tests__/use-router.test.tsx create mode 100644 packages/mosaic/src/routing/__tests__/virtual-location.test.tsx create mode 100644 packages/mosaic/src/routing/location.ts create mode 100644 packages/mosaic/src/routing/router.ts create mode 100644 packages/mosaic/src/routing/routes.test-d.ts create mode 100644 packages/mosaic/src/routing/routes.ts create mode 100644 packages/mosaic/src/routing/routing-scope.tsx create mode 100644 packages/mosaic/src/routing/use-mosaic-location.ts create mode 100644 packages/mosaic/src/routing/use-router.ts create mode 100644 packages/mosaic/src/routing/virtual-location.ts create mode 100644 packages/mosaic/src/routing/virtual-url.tsx diff --git a/packages/mosaic/src/mosaic-provider.tsx b/packages/mosaic/src/mosaic-provider.tsx index 4c8a6748c14..c03e176c8c7 100644 --- a/packages/mosaic/src/mosaic-provider.tsx +++ b/packages/mosaic/src/mosaic-provider.tsx @@ -17,6 +17,7 @@ import type { ModalSurfaces } from './modals/modal.types'; import { ModalHost } from './modals/modal-host'; import type { TextDirection } from './primitives/direction-provider'; import { DirectionProvider } from './primitives/direction-provider'; +import { VirtualUrlProvider } from './routing/virtual-url'; export interface MosaicProviderProps { children: React.ReactNode; @@ -57,10 +58,12 @@ export function MosaicProvider({ - - - {children} - + + + + {children} + + diff --git a/packages/mosaic/src/routing/__tests__/router.test.ts b/packages/mosaic/src/routing/__tests__/router.test.ts new file mode 100644 index 00000000000..463a01b8460 --- /dev/null +++ b/packages/mosaic/src/routing/__tests__/router.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it, vi } from 'vitest'; + +import type { MosaicLocation, SegmentParts } from '../location'; +import { createRouter } from '../router'; + +const routes = { + account: 'account', + security: 'security', + statement: 'billing/statement/:statementId', +} as const; + +function memoryLocation(initial: SegmentParts = []): MosaicLocation & { current: SegmentParts } { + const location = { + current: initial, + useSegmentParts: () => location.current, + write: (parts: SegmentParts) => { + location.current = parts; + }, + }; + return location; +} + +describe('createRouter parse', () => { + const router = createRouter(routes, memoryLocation()); + + it('derives the route from parts', () => { + expect(router.parse(['security'])).toEqual({ name: 'security', params: {} }); + }); + + it('captures params', () => { + expect(router.parse(['billing', 'statement', 'st_1'])).toEqual({ + name: 'statement', + params: { statementId: 'st_1' }, + }); + }); + + it('has no route when the parts match nothing', () => { + expect(router.parse(['nowhere'])).toBeUndefined(); + }); +}); + +describe('createRouter go', () => { + it('writes the built parts to the location, verbatim', () => { + const location = memoryLocation(['account']); + const router = createRouter(routes, location); + + router.go('statement', { statementId: 'st 1' }); + expect(location.current).toEqual(['billing', 'statement', 'st 1']); + }); + + it('passes write options through', () => { + const location = memoryLocation(['account']); + const write = vi.spyOn(location, 'write'); + const router = createRouter(routes, location); + + router.go('security', {}, { replace: true }); + expect(write).toHaveBeenCalledWith(['security'], { replace: true }); + }); +}); diff --git a/packages/mosaic/src/routing/__tests__/routes.test.ts b/packages/mosaic/src/routing/__tests__/routes.test.ts new file mode 100644 index 00000000000..18233a11b86 --- /dev/null +++ b/packages/mosaic/src/routing/__tests__/routes.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it } from 'vitest'; + +import { buildParts, compilePattern, compileRoutes, matchRoute, routeParts } from '../routes'; + +const routes = compileRoutes({ + index: '', + security: 'security', + statement: 'billing/statement/:statementId', +}); + +describe('matchRoute', () => { + it('matches no parts to the index route', () => { + expect(matchRoute(routes, [])).toEqual({ name: 'index', params: {} }); + }); + + it('matches a static route', () => { + expect(matchRoute(routes, ['security'])).toEqual({ name: 'security', params: {} }); + }); + + it('captures params verbatim', () => { + expect(matchRoute(routes, ['billing', 'statement', 'st 1'])).toEqual({ + name: 'statement', + params: { statementId: 'st 1' }, + }); + }); + + it('does not match extra parts', () => { + expect(matchRoute(routes, ['security', 'extra'])).toBeUndefined(); + }); + + it('does not decode a part', () => { + expect(matchRoute(routes, ['billing', 'statement', 'st%201'])).toEqual({ + name: 'statement', + params: { statementId: 'st%201' }, + }); + }); + + it('returns the first route that matches', () => { + expect( + matchRoute(compileRoutes({ plans: 'billing/plans', statement: 'billing/:id' }), ['billing', 'plans']), + ).toEqual({ name: 'plans', params: {} }); + }); +}); + +describe('matchRoute with optional params', () => { + const optional = compileRoutes({ plan: 'billing/plans/:planId?' }); + + it('captures a present optional param', () => { + expect(matchRoute(optional, ['billing', 'plans', 'pro'])).toEqual({ name: 'plan', params: { planId: 'pro' } }); + }); + + it('matches when the optional param is absent', () => { + expect(matchRoute(optional, ['billing', 'plans'])).toEqual({ name: 'plan', params: {} }); + }); + + it('does not match when a required part is missing', () => { + expect(matchRoute(optional, ['billing'])).toBeUndefined(); + }); +}); + +describe('compilePattern', () => { + it('ignores leading, trailing and doubled slashes', () => { + expect(compilePattern('/billing//plans/')).toEqual([ + { param: false, value: 'billing' }, + { param: false, value: 'plans' }, + ]); + }); + + it('rejects a literal that encoding would change', () => { + expect(() => compilePattern('api keys')).toThrow('must be plain'); + expect(() => compilePattern('api%20keys')).toThrow('must be plain'); + }); +}); + +describe('buildParts', () => { + it('returns a static pattern unchanged', () => { + expect(buildParts(compilePattern('security'), {})).toEqual(['security']); + }); + + it('fills params verbatim', () => { + expect(buildParts(compilePattern('billing/statement/:statementId'), { statementId: 'st 1' })).toEqual([ + 'billing', + 'statement', + 'st 1', + ]); + }); + + it('fills a present optional param', () => { + expect(buildParts(compilePattern('billing/plans/:planId?'), { planId: 'pro' })).toEqual([ + 'billing', + 'plans', + 'pro', + ]); + }); + + it('drops an absent optional param', () => { + expect(buildParts(compilePattern('billing/plans/:planId?'), {})).toEqual(['billing', 'plans']); + }); + + it('round-trips any param value through matchRoute', () => { + for (const value of ['st 1', 'st/1', 'st%2F1', '100%', 'ö', '']) { + const parts = buildParts(compilePattern('billing/statement/:statementId'), { statementId: value }); + expect(matchRoute(routes, parts)?.params).toEqual({ statementId: value }); + } + }); +}); + +describe('routeParts', () => { + const table = { account: 'account', apiKeys: 'api-keys', statement: 'billing/statement/:statementId' }; + + it('builds the parts for a route name', () => { + expect(routeParts(table, 'apiKeys')).toEqual(['api-keys']); + expect(routeParts(table, 'statement', { statementId: 'st_1' })).toEqual(['billing', 'statement', 'st_1']); + }); + + it('builds no parts for an unknown name', () => { + expect(routeParts(table, 'nowhere')).toEqual([]); + }); +}); diff --git a/packages/mosaic/src/routing/__tests__/use-router.test.tsx b/packages/mosaic/src/routing/__tests__/use-router.test.tsx new file mode 100644 index 00000000000..a377cc4cb01 --- /dev/null +++ b/packages/mosaic/src/routing/__tests__/use-router.test.tsx @@ -0,0 +1,160 @@ +import { act, renderHook } from '@testing-library/react'; +import type { ReactNode } from 'react'; +import { describe, expect, it } from 'vitest'; + +import type { RouteTable } from '../routes'; +import { MosaicRoutingScope } from '../routing-scope'; +import type { RouterScopeOptions } from '../use-router'; +import { useRouter, useRouterScope } from '../use-router'; +import { useVirtualUrl, useVirtualUrlWrite, VirtualUrlProvider } from '../virtual-url'; + +const routes = { + account: 'account', + security: 'security', + statement: 'billing/statement/:statementId', +} as const; + +const userScope: RouterScopeOptions = { routing: 'virtual', path: '/user', notFound: 'account' }; + +function ScopeOf({ options, children }: { options: RouterScopeOptions; children: ReactNode }) { + const { scope } = useRouterScope(routes, options); + return {children}; +} + +function wrapperAt(initialUrl: string, scopeOptions?: RouterScopeOptions) { + return ({ children }: { children: ReactNode }) => ( + + {scopeOptions ? {children} : children} + + ); +} + +describe('useRouterScope', () => { + it('derives the route from the page virtual URL', () => { + const { result } = renderHook(() => useRouterScope(routes, userScope), { wrapper: wrapperAt('/user/security') }); + expect(result.current.route).toBe('security'); + expect(result.current.params).toEqual({}); + }); + + it('re-renders with the new route after go', () => { + const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), url: useVirtualUrl() }), { + wrapper: wrapperAt('/user'), + }); + + act(() => { + result.current.router.go('statement', { statementId: 'st_1' }); + }); + + expect(result.current.router.route).toBe('statement'); + expect(result.current.router.params).toEqual({ statementId: 'st_1' }); + expect(result.current.url).toBe('/user/billing/statement/st_1'); + }); + + it('encodes on write and decodes on read, exactly once', () => { + const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), url: useVirtualUrl() }), { + wrapper: wrapperAt('/user'), + }); + + act(() => { + result.current.router.go('statement', { statementId: 'st/1 %2F' }); + }); + + expect(result.current.url).toBe('/user/billing/statement/st%2F1%20%252F'); + expect(result.current.router.params).toEqual({ statementId: 'st/1 %2F' }); + }); + + it('shows the not-found route at once and redirects the location to it', () => { + const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), url: useVirtualUrl() }), { + wrapper: wrapperAt('/user/nowhere'), + }); + expect(result.current.router.route).toBe('account'); + expect(result.current.url).toBe('/user/account'); + }); + + it('redirects an empty segment to the not-found route', () => { + const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), url: useVirtualUrl() }), { + wrapper: wrapperAt('/'), + }); + expect(result.current.router.route).toBe('account'); + expect(result.current.url).toBe('/user/account'); + }); + + it('follows a write made to the page virtual URL by someone else', () => { + const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), write: useVirtualUrlWrite() }), { + wrapper: wrapperAt('/user/security'), + }); + + act(() => { + result.current.write('/user/billing/statement/st_2'); + }); + expect(result.current.router.route).toBe('statement'); + }); + + it('keeps the same result object between unrelated renders', () => { + const { result, rerender } = renderHook(() => useRouterScope(routes, userScope), { + wrapper: wrapperAt('/user/security'), + }); + const first = result.current; + rerender(); + expect(result.current).toBe(first); + }); + + it('keeps the router while an equal table is passed as a new object', () => { + const { result, rerender } = renderHook( + ({ table }: { table: RouteTable }) => useRouterScope(table, { ...userScope, notFound: 'account' }), + { initialProps: { table: { ...routes } }, wrapper: wrapperAt('/user/security') }, + ); + const { go } = result.current; + + rerender({ table: { ...routes } }); + expect(result.current.go).toBe(go); + + rerender({ table: { ...routes, usage: 'usage' } }); + expect(result.current.go).not.toBe(go); + act(() => { + result.current.go('usage'); + }); + expect(result.current.route).toBe('usage'); + }); + + it('keeps the same scope while the options are unchanged', () => { + const { result, rerender } = renderHook(() => useRouterScope(routes, { ...userScope }), { + wrapper: wrapperAt('/user/security'), + }); + const { scope } = result.current; + rerender(); + expect(result.current.scope).toBe(scope); + }); +}); + +describe('useRouter', () => { + it('reads the mode and base path from the scope', () => { + const { result } = renderHook(() => useRouter(routes), { wrapper: wrapperAt('/user/security', userScope) }); + expect(result.current.route).toBe('security'); + }); + + it('shares the segment with the owner and other nested routers', () => { + const { result } = renderHook(() => [useRouter(routes), useRouter(routes)] as const, { + wrapper: wrapperAt('/user/security', userScope), + }); + + act(() => { + result.current[0].go('statement', { statementId: 'st_1' }); + }); + + expect(result.current[1].route).toBe('statement'); + }); + + it('reports no route for a segment a sibling owns, without redirecting', () => { + const billing = { statement: 'billing/statement/:statementId' } as const; + const { result } = renderHook(() => ({ router: useRouter(billing), url: useVirtualUrl() }), { + wrapper: wrapperAt('/user/security', userScope), + }); + expect(result.current.router.route).toBeUndefined(); + expect(result.current.url).toBe('/user/security'); + }); + + it('throws without a scope', () => { + expect(() => renderHook(() => useRouter(routes), { wrapper: wrapperAt('/user') })).toThrow('useRouterScope'); + }); +}); diff --git a/packages/mosaic/src/routing/__tests__/virtual-location.test.tsx b/packages/mosaic/src/routing/__tests__/virtual-location.test.tsx new file mode 100644 index 00000000000..3980d25db0f --- /dev/null +++ b/packages/mosaic/src/routing/__tests__/virtual-location.test.tsx @@ -0,0 +1,124 @@ +import { act, renderHook } from '@testing-library/react'; +import type { ReactNode } from 'react'; +import { describe, expect, it } from 'vitest'; + +import { + createVirtualLocation, + decodeParts, + encodeParts, + normalizeBase, + rawSegmentPartsOf, + urlFor, +} from '../virtual-location'; +import { useVirtualUrl, useVirtualUrlWrite, VirtualUrlProvider } from '../virtual-url'; + +describe('normalizeBase', () => { + it('strips surrounding slashes and collapses the root', () => { + expect(normalizeBase('/user/')).toBe('/user'); + expect(normalizeBase('user')).toBe('/user'); + expect(normalizeBase('/')).toBe(''); + expect(normalizeBase('')).toBe(''); + }); +}); + +describe('rawSegmentPartsOf', () => { + it('reads the raw parts under the base', () => { + expect(rawSegmentPartsOf('/user/security', '/user')).toEqual(['security']); + expect(rawSegmentPartsOf('/user/billing/statement/st%2F1', '/user')).toEqual(['billing', 'statement', 'st%2F1']); + }); + + it('reads no parts at the base', () => { + expect(rawSegmentPartsOf('/user', '/user')).toEqual([]); + expect(rawSegmentPartsOf('/user/', '/user')).toEqual([]); + }); + + it('reads no parts outside the base', () => { + expect(rawSegmentPartsOf('/sign-in/factor-one', '/user')).toEqual([]); + expect(rawSegmentPartsOf('/users', '/user')).toEqual([]); + }); + + it('reads the whole path at the root base', () => { + expect(rawSegmentPartsOf('/security', '')).toEqual(['security']); + expect(rawSegmentPartsOf('/', '')).toEqual([]); + }); +}); + +describe('urlFor', () => { + it('joins the base and the raw parts', () => { + expect(urlFor('/user', ['security'])).toBe('/user/security'); + expect(urlFor('/user', [])).toBe('/user'); + expect(urlFor('', ['security'])).toBe('/security'); + expect(urlFor('', [])).toBe('/'); + }); +}); + +describe('decodeParts and encodeParts', () => { + it('round-trip any value', () => { + const parts = ['st/1', '100%', 'a b', 'ö', '']; + expect(decodeParts(encodeParts(parts))).toEqual(parts); + }); + + it('report no parts for a malformed escape', () => { + expect(decodeParts(['billing', '%E0%A4'])).toEqual([]); + }); +}); + +function wrapperAt(initialUrl: string) { + return ({ children }: { children: ReactNode }) => ( + {children} + ); +} + +function useLocationAt(basePath: string) { + const location = createVirtualLocation(basePath, useVirtualUrlWrite()); + return { location, parts: location.useSegmentParts(), url: useVirtualUrl() }; +} + +describe('createVirtualLocation', () => { + it('derives decoded parts from the page virtual URL', () => { + const { result } = renderHook(() => useLocationAt('/user'), { wrapper: wrapperAt('/user/billing/st%2F1') }); + expect(result.current.parts).toEqual(['billing', 'st/1']); + }); + + it('writes encoded parts under its base', () => { + const { result } = renderHook(() => useLocationAt('/user'), { wrapper: wrapperAt('/') }); + + act(() => { + result.current.location.write(['billing', 'statement', 'st/1 %']); + }); + + expect(result.current.url).toBe('/user/billing/statement/st%2F1%20%25'); + expect(result.current.parts).toEqual(['billing', 'statement', 'st/1 %']); + }); + + it('returns the same parts object while the URL is unchanged', () => { + const { result, rerender } = renderHook(() => useLocationAt('/user'), { wrapper: wrapperAt('/user/security') }); + const first = result.current.parts; + rerender(); + expect(result.current.parts).toBe(first); + }); + + it('reports no parts for a malformed segment', () => { + const { result } = renderHook(() => useLocationAt('/user'), { wrapper: wrapperAt('/user/%E0%A4') }); + expect(result.current.parts).toEqual([]); + }); + + it('shares the page virtual URL between instances with different bases', () => { + const { result } = renderHook(() => [useLocationAt('/user'), useLocationAt('/sign-in')] as const, { + wrapper: wrapperAt('/user/security'), + }); + expect(result.current[0].parts).toEqual(['security']); + expect(result.current[1].parts).toEqual([]); + + act(() => { + result.current[1].location.write(['factor-one']); + }); + + expect(result.current[0].parts).toEqual([]); + expect(result.current[1].parts).toEqual(['factor-one']); + }); + + it('throws without a provider', () => { + expect(() => renderHook(() => useLocationAt('/user'))).toThrow(''); + }); +}); diff --git a/packages/mosaic/src/routing/location.ts b/packages/mosaic/src/routing/location.ts new file mode 100644 index 00000000000..9a6c9014b5b --- /dev/null +++ b/packages/mosaic/src/routing/location.ts @@ -0,0 +1,14 @@ +export interface WriteOptions { + replace?: boolean; +} + +export type SegmentParts = readonly string[]; + +// A location owns the encoding boundary. The parts it hands out are decoded exactly once, per part, +// as close to the source as the source allows; the parts it is given are encoded on write the same +// way. Nothing above a location encodes or decodes a route part again. `useSegmentParts` returns the +// same array for as long as the segment is unchanged, so callers can memoise on it. +export interface MosaicLocation { + useSegmentParts: () => SegmentParts; + write: (parts: SegmentParts, options?: WriteOptions) => void; +} diff --git a/packages/mosaic/src/routing/router.ts b/packages/mosaic/src/routing/router.ts new file mode 100644 index 00000000000..de73fa17646 --- /dev/null +++ b/packages/mosaic/src/routing/router.ts @@ -0,0 +1,44 @@ +import type { MosaicLocation, SegmentParts, WriteOptions } from './location'; +import { + buildParts, + compileRoutes, + matchRoute, + type PatternPart, + type RouteName, + type RouteParams, + type RouteTable, +} from './routes'; + +export interface Route { + name: RouteName; + params: Record; +} + +type RouteArgs = + Record extends RouteParams + ? [params?: RouteParams, rest?: Rest] + : [params: RouteParams, rest?: Rest]; + +export interface Router { + parse: (parts: SegmentParts) => Route | undefined; + go: >(name: Name, ...args: RouteArgs) => void; +} + +export function createRouter( + routes: Routes, + location: MosaicLocation, +): Router { + const compiled = compileRoutes(routes); + const patternsByName = new Map(compiled.map(route => [route.name, route.parts])); + + return { + parse: parts => { + const match = matchRoute(compiled, parts); + return match ? { name: match.name, params: match.params } : undefined; + }, + go: (name, ...args) => { + const [params = {}, options] = args; + location.write(buildParts(patternsByName.get(name) ?? [], params), options); + }, + }; +} diff --git a/packages/mosaic/src/routing/routes.test-d.ts b/packages/mosaic/src/routing/routes.test-d.ts new file mode 100644 index 00000000000..f63b2f3e61f --- /dev/null +++ b/packages/mosaic/src/routing/routes.test-d.ts @@ -0,0 +1,38 @@ +import { describe, expectTypeOf, test } from 'vitest'; + +import type { MosaicLocation } from './location'; +import { createRouter } from './router'; +import type { RouteParams } from './routes'; + +describe('RouteParams', () => { + test('infers required and optional params from the pattern', () => { + expectTypeOf>().toEqualTypeOf< + Record<'orgId', string> & Partial> + >(); + }); + + test('infers no params for a static pattern', () => { + expectTypeOf>().toEqualTypeOf(); + }); +}); + +describe('Router', () => { + const location: MosaicLocation = { useSegmentParts: () => [], write: () => {} }; + const router = createRouter( + { index: '', security: 'security', statement: 'billing/statement/:statementId' }, + location, + ); + + test('requires the params a pattern declares', () => { + router.go('security'); + router.go('statement', { statementId: 'st_1' }); + // @ts-expect-error statementId is required + router.go('statement'); + // @ts-expect-error unknown route + router.go('billing'); + }); + + test('narrows the route name', () => { + expectTypeOf(router.parse([])?.name).toEqualTypeOf<'index' | 'security' | 'statement' | undefined>(); + }); +}); diff --git a/packages/mosaic/src/routing/routes.ts b/packages/mosaic/src/routing/routes.ts new file mode 100644 index 00000000000..5d55d71ab12 --- /dev/null +++ b/packages/mosaic/src/routing/routes.ts @@ -0,0 +1,160 @@ +// The codec between route names and the parts of the segment a component owns. It is pure and +// representation-free: it works on decoded parts, never on a URL string, and never encodes or +// decodes anything. That is the location's job. +// +// The shapes, end to end: +// table { statement: 'billing/statement/:id' } names to patterns, what a surface declares +// compiled { name: 'statement', parts: [...], minLength: 3 } one entry per route, ready to match +// parts ['billing', 'statement', 'st_1'] the decoded segment a location hands over +// match { name: 'statement', params: { id: 'st_1' } } what the surface renders from +// +// matchRoute goes parts -> match. buildParts and routeParts go name + params -> parts. + +// { statement: 'billing/statement/:id' }. A pattern is relative to the component's base; `:name` captures a part, `:name?` may be absent. +export type RouteTable = Readonly>; + +export type RouteName = Extract; + +// 'billing/statement/:id' -> 'billing' | 'statement' | ':id' +type Parts = Pattern extends `${infer Part}/${infer Rest}` ? Part | Parts : Pattern; + +type OptionalParamNames = Part extends `:${infer Name}?` ? Name : never; + +type RequiredParamNames = Part extends `:${string}?` + ? never + : Part extends `:${infer Name}` + ? Name + : never; + +// 'org/:orgId/member/:memberId?' -> { orgId: string; memberId?: string } +export type RouteParams = Record>, string> & + Partial>, string>>; + +export interface RouteMatch { + name: Name; + params: Record; +} + +// 'billing' -> { param: false, value: 'billing' } ':id?' -> { param: true, name: 'id', optional: true } +export type PatternPart = { param: false; value: string } | { param: true; name: string; optional: boolean }; + +// minLength is how many parts a segment must have to match: up to the last non-optional part. +export interface CompiledRoute { + name: Name; + parts: readonly PatternPart[]; + minLength: number; +} + +// '/billing//plans/' -> ['billing', 'plans'] +function partsOf(pattern: string): string[] { + return pattern.split('/').filter(Boolean); +} + +// Literals are compared against decoded parts, so 'api keys' or 'api%20keys' could never match the way they read. +function assertPlainLiteral(part: string): void { + if (encodeURIComponent(part) !== part) { + throw new Error(`[clerk] Route pattern part "${part}" must be plain; patterns are matched against decoded parts.`); + } +} + +// 'billing/plans/:planId?' -> [{ param: false, value: 'billing' }, { param: false, value: 'plans' }, { param: true, name: 'planId', optional: true }] +export function compilePattern(pattern: string): PatternPart[] { + return partsOf(pattern).map(part => { + if (!part.startsWith(':')) { + assertPlainLiteral(part); + return { param: false, value: part }; + } + const optional = part.endsWith('?'); + return { param: true, name: part.slice(1, optional ? -1 : undefined), optional }; + }); +} + +// 'billing/plans/:planId?' -> 2 'billing/statement/:id' -> 3 +function minLengthOf(parts: readonly PatternPart[]): number { + let minLength = 0; + for (const [index, part] of parts.entries()) { + if (!part.param || !part.optional) { + minLength = index + 1; + } + } + return minLength; +} + +function isRouteName(routes: Routes, name: string): name is RouteName { + return Object.prototype.hasOwnProperty.call(routes, name); +} + +// { security: 'security', statement: 'billing/statement/:id' } -> [compiled security, compiled statement] +// Declaration order is match precedence. +export function compileRoutes(routes: Routes): CompiledRoute>[] { + const compiled: CompiledRoute>[] = []; + for (const [name, pattern] of Object.entries(routes)) { + if (isRouteName(routes, name)) { + const parts = compilePattern(pattern); + compiled.push({ name, parts, minLength: minLengthOf(parts) }); + } + } + return compiled; +} + +// compiled 'billing/statement/:id' + ['billing', 'statement', 'st_1'] -> { id: 'st_1' } +// Literals must be equal, params capture verbatim, the part count must fit between minLength and the full length. +function matchParts(route: CompiledRoute, parts: readonly string[]): Record | undefined { + if (parts.length < route.minLength || parts.length > route.parts.length) { + return undefined; + } + const params: Record = {}; + for (const [index, part] of parts.entries()) { + const patternPart = route.parts[index]; + if (!patternPart) { + return undefined; + } + if (patternPart.param) { + params[patternPart.name] = part; + } else if (patternPart.value !== part) { + return undefined; + } + } + return params; +} + +// ['billing', 'statement', 'st_1'] -> { name: 'statement', params: { id: 'st_1' } }, or undefined when no route fits +export function matchRoute( + routes: readonly CompiledRoute[], + parts: readonly string[], +): RouteMatch | undefined { + for (const route of routes) { + const params = matchParts(route, parts); + if (params) { + return { name: route.name, params }; + } + } + return undefined; +} + +// (table, 'statement', { id: 'st_1' }) -> ['billing', 'statement', 'st_1'] unknown name -> [] +// For callers that have a table but no router. +export function routeParts( + routes: RouteTable, + name: string, + params: Record = {}, +): string[] { + const pattern = routes[name]; + return pattern === undefined ? [] : buildParts(compilePattern(pattern), params); +} + +// compiled 'billing/plans/:planId?' + { planId: 'pro' } -> ['billing', 'plans', 'pro'] + {} -> ['billing', 'plans'] +export function buildParts(parts: readonly PatternPart[], params: Record): string[] { + const built: string[] = []; + for (const part of parts) { + if (!part.param) { + built.push(part.value); + continue; + } + const value = params[part.name]; + if (value !== undefined) { + built.push(value); + } + } + return built; +} diff --git a/packages/mosaic/src/routing/routing-scope.tsx b/packages/mosaic/src/routing/routing-scope.tsx new file mode 100644 index 00000000000..6372b934c2d --- /dev/null +++ b/packages/mosaic/src/routing/routing-scope.tsx @@ -0,0 +1,31 @@ +'use client'; + +import type { ReactNode } from 'react'; +import { createContext, useContext } from 'react'; + +import type { MosaicLocationOptions } from './use-mosaic-location'; + +const scopeBrand: unique symbol = Symbol('MosaicRoutingScope'); + +export interface RoutingScope { + readonly [scopeBrand]: MosaicLocationOptions; +} + +export function createRoutingScope(options: MosaicLocationOptions): RoutingScope { + return { [scopeBrand]: options }; +} + +export interface MosaicRoutingScopeProps { + scope: RoutingScope; + children: ReactNode; +} + +const MosaicRoutingScopeContext = createContext(undefined); + +export function MosaicRoutingScope({ scope, children }: MosaicRoutingScopeProps) { + return {children}; +} + +export function useInheritedScope(): MosaicLocationOptions | undefined { + return useContext(MosaicRoutingScopeContext)?.[scopeBrand]; +} diff --git a/packages/mosaic/src/routing/use-mosaic-location.ts b/packages/mosaic/src/routing/use-mosaic-location.ts new file mode 100644 index 00000000000..0df2ae80217 --- /dev/null +++ b/packages/mosaic/src/routing/use-mosaic-location.ts @@ -0,0 +1,20 @@ +import { useMemo } from 'react'; + +import type { MosaicLocation } from './location'; +import { createVirtualLocation } from './virtual-location'; +import { useVirtualUrlWrite } from './virtual-url'; + +export type MosaicLocationOptions = { routing: 'virtual'; path?: string }; + +export interface RoutingProps { + /** Where the component's pages live: `virtual` keeps them in memory. Defaults per component. */ + routing?: MosaicLocationOptions['routing']; + /** The base the component's pages are nested under. Defaults per component. */ + path?: string; +} + +export function useMosaicLocation(options: MosaicLocationOptions): MosaicLocation { + const writeVirtualUrl = useVirtualUrlWrite(); + const path = options.path ?? '/'; + return useMemo(() => createVirtualLocation(path, writeVirtualUrl), [path, writeVirtualUrl]); +} diff --git a/packages/mosaic/src/routing/use-router.ts b/packages/mosaic/src/routing/use-router.ts new file mode 100644 index 00000000000..66cf39b7722 --- /dev/null +++ b/packages/mosaic/src/routing/use-router.ts @@ -0,0 +1,109 @@ +import { useEffect, useMemo, useState } from 'react'; + +import type { MosaicLocation } from './location'; +import type { Route, Router } from './router'; +import { createRouter } from './router'; +import type { RouteName, RouteTable } from './routes'; +import { routeParts } from './routes'; +import type { RoutingScope } from './routing-scope'; +import { createRoutingScope, useInheritedScope } from './routing-scope'; +import type { MosaicLocationOptions } from './use-mosaic-location'; +import { useMosaicLocation } from './use-mosaic-location'; + +const EMPTY_PARAMS: Record = Object.freeze({}); + +const KEY_SEPARATOR = '\u0000'; + +// Keeps the same value object for as long as its key is unchanged, so memoisation downstream holds +// across callers that build a new object every render. Setting state during render makes React +// restart the render with the new value when the key changes, which is the documented way to derive +// state from props. +function useStableByKey(value: Value, key: string): Value { + const [stable, setStable] = useState({ key, value }); + if (stable.key !== key) { + setStable({ key, value }); + } + return stable.key === key ? stable.value : value; +} + +function useStableTable(routes: Routes): Routes { + return useStableByKey(routes, Object.entries(routes).flat().join(KEY_SEPARATOR)); +} + +function missingScope(): never { + throw new Error('[clerk] useRouter needs a above it; the segment owner calls useRouterScope.'); +} + +interface Routing { + location: MosaicLocation; + table: Routes; + router: Router; + matched: Route | undefined; +} + +function useRouting(routes: Routes, options: MosaicLocationOptions): Routing { + const location = useMosaicLocation(options); + const table = useStableTable(routes); + const router = useMemo(() => createRouter(table, location), [table, location]); + const parts = location.useSegmentParts(); + const matched = useMemo(() => router.parse(parts), [router, parts]); + return { location, table, router, matched }; +} + +export interface RouterScopeOptions { + routing: MosaicLocationOptions['routing']; + path?: string; + // The route a segment no route matches redirects to, with replace. The owner of a segment always + // decides what an unknown segment means, so `route` below is never undefined. + notFound: RouteName; +} + +export interface RouterScope { + route: RouteName; + params: Record; + go: Router['go']; + scope: RoutingScope; +} + +// For the component that owns a segment of the location. It creates the scope nested routers read, +// and it is the only router that redirects. +export function useRouterScope( + routes: Routes, + { routing, path, notFound }: RouterScopeOptions, +): RouterScope { + const scope = useMemo(() => createRoutingScope({ routing, path }), [routing, path]); + const { location, table, router, matched } = useRouting(routes, { routing, path }); + const current = useMemo>( + () => matched ?? { name: notFound, params: EMPTY_PARAMS }, + [matched, notFound], + ); + + const redirectTo = matched === undefined ? notFound : undefined; + useEffect(() => { + if (redirectTo !== undefined) { + location.write(routeParts(table, redirectTo), { replace: true }); + } + }, [redirectTo, location, table]); + + return useMemo( + () => ({ route: current.name, params: current.params, go: router.go, scope }), + [router, scope, current], + ); +} + +export interface NestedRouter { + route: RouteName | undefined; + params: Record; + go: Router['go']; +} + +// For anything rendered inside a segment owner. It matches its own table against the segment and +// reports no route when the segment belongs to a sibling; it never redirects. +export function useRouter(routes: Routes): NestedRouter { + const { router, matched } = useRouting(routes, useInheritedScope() ?? missingScope()); + + return useMemo( + () => ({ route: matched?.name, params: matched?.params ?? EMPTY_PARAMS, go: router.go }), + [router, matched], + ); +} diff --git a/packages/mosaic/src/routing/virtual-location.ts b/packages/mosaic/src/routing/virtual-location.ts new file mode 100644 index 00000000000..9404b1a120d --- /dev/null +++ b/packages/mosaic/src/routing/virtual-location.ts @@ -0,0 +1,57 @@ +import { useMemo } from 'react'; + +import type { MosaicLocation, SegmentParts } from './location'; +import { useVirtualUrl } from './virtual-url'; + +// The virtual URL is kept in its encoded form, like a real one, so all locations work the same + +export function normalizeBase(base: string): string { + const parts = base.split('/').filter(Boolean); + return parts.length ? `/${parts.join('/')}` : ''; +} + +function rawPartsOf(url: string): string[] { + return url.split('/').filter(Boolean); +} + +export function rawSegmentPartsOf(url: string, base: string): string[] { + const parts = rawPartsOf(url); + const baseParts = rawPartsOf(base); + if (baseParts.length > parts.length || baseParts.some((part, index) => parts[index] !== part)) { + return []; + } + return parts.slice(baseParts.length); +} + +export function urlFor(base: string, rawParts: readonly string[]): string { + return rawParts.length ? `${base}/${rawParts.join('/')}` : base || '/'; +} + +const EMPTY_PARTS: SegmentParts = Object.freeze([]); + +// A malformed escape means the segment cannot be read, so the location reports no parts rather +// than a half-decoded value that something downstream might decode again. +export function decodeParts(rawParts: readonly string[]): SegmentParts { + try { + return rawParts.map(part => decodeURIComponent(part)); + } catch { + return EMPTY_PARTS; + } +} + +export function encodeParts(parts: SegmentParts): string[] { + return parts.map(part => encodeURIComponent(part)); +} + +function useVirtualSegmentParts(base: string): SegmentParts { + const url = useVirtualUrl(); + return useMemo(() => decodeParts(rawSegmentPartsOf(url, base)), [url, base]); +} + +export function createVirtualLocation(basePath: string, write: (url: string) => void): MosaicLocation { + const base = normalizeBase(basePath); + return { + useSegmentParts: () => useVirtualSegmentParts(base), + write: parts => write(urlFor(base, encodeParts(parts))), + }; +} diff --git a/packages/mosaic/src/routing/virtual-url.tsx b/packages/mosaic/src/routing/virtual-url.tsx new file mode 100644 index 00000000000..7ccc356bd39 --- /dev/null +++ b/packages/mosaic/src/routing/virtual-url.tsx @@ -0,0 +1,33 @@ +'use client'; + +import type { ReactNode } from 'react'; +import { createContext, useContext, useState } from 'react'; + +const VirtualUrlValueContext = createContext(undefined); +const VirtualUrlWriteContext = createContext<((url: string) => void) | undefined>(undefined); + +export interface VirtualUrlProviderProps { + initialUrl?: string; + children: ReactNode; +} + +export function VirtualUrlProvider({ initialUrl = '/', children }: VirtualUrlProviderProps) { + const [url, setUrl] = useState(initialUrl); + return ( + + {children} + + ); +} + +function missingProvider(): never { + throw new Error('[clerk] Virtual routing must be used within a .'); +} + +export function useVirtualUrl(): string { + return useContext(VirtualUrlValueContext) ?? missingProvider(); +} + +export function useVirtualUrlWrite(): (url: string) => void { + return useContext(VirtualUrlWriteContext) ?? missingProvider(); +} From 6b9360ee89887ce50fd35999795dcac3436e80e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Tue, 3 Nov 2026 14:52:58 +0100 Subject: [PATCH 02/26] Drive profiles via virtual router instead of useState --- .../organization-profile.tsx | 6 +++-- .../profile-pages/profile-pages.controller.ts | 25 ++++++++----------- .../features/user-profile/user-profile.tsx | 20 ++++++++------- 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.tsx b/packages/mosaic/src/features/organization-profile/organization-profile.tsx index c4d69f58f5f..a0793018727 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.tsx +++ b/packages/mosaic/src/features/organization-profile/organization-profile.tsx @@ -92,9 +92,10 @@ export function OrganizationProfile(props: OrganizationProfileProps) { } = props; const model = useOrganizationProfileModel(); const pages = model.status === 'ready' ? organizationProfilePanels(model.pages, apiKeysProps) : undefined; - const navigation = useProfilePagesController( + const { activePage, onPageChange } = useProfilePagesController( defaultPage ?? pageOrder?.[0] ?? 'general', pages && organizationProfilePageIds({ pages, customPages, pageOrder }), + { routing: 'virtual', path: '/organization' }, ); if (model.status === 'loading') { @@ -108,7 +109,8 @@ export function OrganizationProfile(props: OrganizationProfileProps) { [id, id])), { + ...routing, + notFound: fallbackPage, + }); - if (pageIds) { - if (isOpening) { - setIsOpening(false); - } - const fallbackPage = (isOpening ? openingFallbackPage : undefined) ?? pageIds[0]; - if (fallbackPage !== undefined && !pageIds.includes(activePage)) { - setActivePage(fallbackPage); - } - } - - return { activePage, onPageChange: setActivePage }; + return { activePage: route, onPageChange: go }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile.tsx b/packages/mosaic/src/features/user-profile/user-profile.tsx index 754efaa863e..9b82d833dc1 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile.tsx @@ -206,12 +206,16 @@ type AccountViewProps = Omit ); } @@ -226,20 +230,18 @@ function CombinedUserProfileView({ model.status === 'ready' ? { pages: organizationProfilePanels(model.pages, apiKeysProps), customPages, pageOrder } : undefined; - const pageIds = model.status === 'loading' ? undefined : userProfilePageIds({ ...viewProps, organization }); - const navigation = useProfilePagesController( + const { activePage, onPageChange } = useProfilePagesController( initialPage, - pageIds, - initialPage.startsWith(ORGANIZATION_PAGE_PREFIX) - ? pageIds?.find(id => id.startsWith(ORGANIZATION_PAGE_PREFIX)) - : undefined, + model.status === 'loading' ? undefined : userProfilePageIds({ ...viewProps, organization }), + { routing: 'virtual', path: '/user' }, ); return ( From 310d0d1f5d858566fc0da0789b71f3032814a0c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Wed, 4 Nov 2026 15:11:36 +0100 Subject: [PATCH 03/26] Make VirtualUrlProvider->VirtualStoreProvider to handle multiple bases --- packages/mosaic/src/mosaic-provider.tsx | 6 +- .../src/routing/__tests__/router.test.ts | 3 + .../src/routing/__tests__/use-router.test.tsx | 112 +++++++++++----- .../__tests__/virtual-location.test.tsx | 126 ++++++++---------- packages/mosaic/src/routing/base-path.ts | 5 + packages/mosaic/src/routing/location.ts | 12 +- packages/mosaic/src/routing/routes.test-d.ts | 2 +- .../mosaic/src/routing/use-mosaic-location.ts | 6 +- .../mosaic/src/routing/virtual-location.ts | 61 ++------- packages/mosaic/src/routing/virtual-store.tsx | 78 +++++++++++ packages/mosaic/src/routing/virtual-url.tsx | 33 ----- 11 files changed, 239 insertions(+), 205 deletions(-) create mode 100644 packages/mosaic/src/routing/base-path.ts create mode 100644 packages/mosaic/src/routing/virtual-store.tsx delete mode 100644 packages/mosaic/src/routing/virtual-url.tsx diff --git a/packages/mosaic/src/mosaic-provider.tsx b/packages/mosaic/src/mosaic-provider.tsx index c03e176c8c7..69f0cec5c45 100644 --- a/packages/mosaic/src/mosaic-provider.tsx +++ b/packages/mosaic/src/mosaic-provider.tsx @@ -17,7 +17,7 @@ import type { ModalSurfaces } from './modals/modal.types'; import { ModalHost } from './modals/modal-host'; import type { TextDirection } from './primitives/direction-provider'; import { DirectionProvider } from './primitives/direction-provider'; -import { VirtualUrlProvider } from './routing/virtual-url'; +import { VirtualStoreProvider } from './routing/virtual-store'; export interface MosaicProviderProps { children: React.ReactNode; @@ -58,12 +58,12 @@ export function MosaicProvider({ - + {children} - + diff --git a/packages/mosaic/src/routing/__tests__/router.test.ts b/packages/mosaic/src/routing/__tests__/router.test.ts index 463a01b8460..0c3c5f5eaac 100644 --- a/packages/mosaic/src/routing/__tests__/router.test.ts +++ b/packages/mosaic/src/routing/__tests__/router.test.ts @@ -16,6 +16,9 @@ function memoryLocation(initial: SegmentParts = []): MosaicLocation & { current: write: (parts: SegmentParts) => { location.current = parts; }, + clear: () => { + location.current = []; + }, }; return location; } diff --git a/packages/mosaic/src/routing/__tests__/use-router.test.tsx b/packages/mosaic/src/routing/__tests__/use-router.test.tsx index a377cc4cb01..895bbabd779 100644 --- a/packages/mosaic/src/routing/__tests__/use-router.test.tsx +++ b/packages/mosaic/src/routing/__tests__/use-router.test.tsx @@ -2,11 +2,12 @@ import { act, renderHook } from '@testing-library/react'; import type { ReactNode } from 'react'; import { describe, expect, it } from 'vitest'; +import type { SegmentParts } from '../location'; import type { RouteTable } from '../routes'; import { MosaicRoutingScope } from '../routing-scope'; import type { RouterScopeOptions } from '../use-router'; import { useRouter, useRouterScope } from '../use-router'; -import { useVirtualUrl, useVirtualUrlWrite, VirtualUrlProvider } from '../virtual-url'; +import { useVirtualSegment, useVirtualWrite, VirtualStoreProvider } from '../virtual-store'; const routes = { account: 'account', @@ -21,24 +22,28 @@ function ScopeOf({ options, children }: { options: RouterScopeOptions{children}; } -function wrapperAt(initialUrl: string, scopeOptions?: RouterScopeOptions) { +function wrapperWith(initial?: Record, scopeOptions?: RouterScopeOptions) { return ({ children }: { children: ReactNode }) => ( - + {scopeOptions ? {children} : children} - + ); } +const useUserSegment = () => useVirtualSegment('/user'); + describe('useRouterScope', () => { - it('derives the route from the page virtual URL', () => { - const { result } = renderHook(() => useRouterScope(routes, userScope), { wrapper: wrapperAt('/user/security') }); + it('derives the route from its entry', () => { + const { result } = renderHook(() => useRouterScope(routes, userScope), { + wrapper: wrapperWith({ '/user': ['security'] }), + }); expect(result.current.route).toBe('security'); expect(result.current.params).toEqual({}); }); it('re-renders with the new route after go', () => { - const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), url: useVirtualUrl() }), { - wrapper: wrapperAt('/user'), + const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), segment: useUserSegment() }), { + wrapper: wrapperWith({ '/user': ['account'] }), }); act(() => { @@ -47,52 +52,85 @@ describe('useRouterScope', () => { expect(result.current.router.route).toBe('statement'); expect(result.current.router.params).toEqual({ statementId: 'st_1' }); - expect(result.current.url).toBe('/user/billing/statement/st_1'); + expect(result.current.segment).toEqual(['billing', 'statement', 'st_1']); }); - it('encodes on write and decodes on read, exactly once', () => { - const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), url: useVirtualUrl() }), { - wrapper: wrapperAt('/user'), + it('round-trips any param value', () => { + const { result } = renderHook(() => useRouterScope(routes, userScope), { + wrapper: wrapperWith({ '/user': ['account'] }), }); act(() => { - result.current.router.go('statement', { statementId: 'st/1 %2F' }); + result.current.go('statement', { statementId: 'st/1 %2F' }); }); - expect(result.current.url).toBe('/user/billing/statement/st%2F1%20%252F'); - expect(result.current.router.params).toEqual({ statementId: 'st/1 %2F' }); + expect(result.current.params).toEqual({ statementId: 'st/1 %2F' }); }); - it('shows the not-found route at once and redirects the location to it', () => { - const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), url: useVirtualUrl() }), { - wrapper: wrapperAt('/user/nowhere'), + it('shows the not-found route at once and redirects its entry to it', () => { + const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), segment: useUserSegment() }), { + wrapper: wrapperWith({ '/user': ['nowhere'] }), }); expect(result.current.router.route).toBe('account'); - expect(result.current.url).toBe('/user/account'); + expect(result.current.segment).toEqual(['account']); }); - it('redirects an empty segment to the not-found route', () => { - const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), url: useVirtualUrl() }), { - wrapper: wrapperAt('/'), + it('redirects an empty entry to the not-found route', () => { + const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), segment: useUserSegment() }), { + wrapper: wrapperWith(), }); expect(result.current.router.route).toBe('account'); - expect(result.current.url).toBe('/user/account'); + expect(result.current.segment).toEqual(['account']); }); - it('follows a write made to the page virtual URL by someone else', () => { - const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), write: useVirtualUrlWrite() }), { - wrapper: wrapperAt('/user/security'), + it('follows a write made to its entry by someone else', () => { + const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), write: useVirtualWrite() }), { + wrapper: wrapperWith({ '/user': ['security'] }), }); act(() => { - result.current.write('/user/billing/statement/st_2'); + result.current.write('/user', ['billing', 'statement', 'st_2']); }); expect(result.current.router.route).toBe('statement'); }); + it('ignores writes to other entries', () => { + const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), write: useVirtualWrite() }), { + wrapper: wrapperWith({ '/user': ['security'] }), + }); + + act(() => { + result.current.write('/organization', ['members']); + }); + expect(result.current.router.route).toBe('security'); + }); + + it('lets two owners with different bases coexist', () => { + const orgRoutes = { general: 'general', members: 'members' } as const; + const { result } = renderHook( + () => ({ + user: useRouterScope(routes, userScope), + org: useRouterScope(orgRoutes, { routing: 'virtual', path: '/organization', notFound: 'general' }), + }), + { wrapper: wrapperWith() }, + ); + expect(result.current.user.route).toBe('account'); + expect(result.current.org.route).toBe('general'); + + act(() => { + result.current.user.go('security'); + }); + act(() => { + result.current.org.go('members'); + }); + + expect(result.current.user.route).toBe('security'); + expect(result.current.org.route).toBe('members'); + }); + it('keeps the same result object between unrelated renders', () => { const { result, rerender } = renderHook(() => useRouterScope(routes, userScope), { - wrapper: wrapperAt('/user/security'), + wrapper: wrapperWith({ '/user': ['security'] }), }); const first = result.current; rerender(); @@ -102,7 +140,7 @@ describe('useRouterScope', () => { it('keeps the router while an equal table is passed as a new object', () => { const { result, rerender } = renderHook( ({ table }: { table: RouteTable }) => useRouterScope(table, { ...userScope, notFound: 'account' }), - { initialProps: { table: { ...routes } }, wrapper: wrapperAt('/user/security') }, + { initialProps: { table: { ...routes } }, wrapper: wrapperWith({ '/user': ['security'] }) }, ); const { go } = result.current; @@ -119,7 +157,7 @@ describe('useRouterScope', () => { it('keeps the same scope while the options are unchanged', () => { const { result, rerender } = renderHook(() => useRouterScope(routes, { ...userScope }), { - wrapper: wrapperAt('/user/security'), + wrapper: wrapperWith({ '/user': ['security'] }), }); const { scope } = result.current; rerender(); @@ -129,13 +167,15 @@ describe('useRouterScope', () => { describe('useRouter', () => { it('reads the mode and base path from the scope', () => { - const { result } = renderHook(() => useRouter(routes), { wrapper: wrapperAt('/user/security', userScope) }); + const { result } = renderHook(() => useRouter(routes), { + wrapper: wrapperWith({ '/user': ['security'] }, userScope), + }); expect(result.current.route).toBe('security'); }); it('shares the segment with the owner and other nested routers', () => { const { result } = renderHook(() => [useRouter(routes), useRouter(routes)] as const, { - wrapper: wrapperAt('/user/security', userScope), + wrapper: wrapperWith({ '/user': ['security'] }, userScope), }); act(() => { @@ -147,14 +187,14 @@ describe('useRouter', () => { it('reports no route for a segment a sibling owns, without redirecting', () => { const billing = { statement: 'billing/statement/:statementId' } as const; - const { result } = renderHook(() => ({ router: useRouter(billing), url: useVirtualUrl() }), { - wrapper: wrapperAt('/user/security', userScope), + const { result } = renderHook(() => ({ router: useRouter(billing), segment: useUserSegment() }), { + wrapper: wrapperWith({ '/user': ['security'] }, userScope), }); expect(result.current.router.route).toBeUndefined(); - expect(result.current.url).toBe('/user/security'); + expect(result.current.segment).toEqual(['security']); }); it('throws without a scope', () => { - expect(() => renderHook(() => useRouter(routes), { wrapper: wrapperAt('/user') })).toThrow('useRouterScope'); + expect(() => renderHook(() => useRouter(routes), { wrapper: wrapperWith() })).toThrow('useRouterScope'); }); }); diff --git a/packages/mosaic/src/routing/__tests__/virtual-location.test.tsx b/packages/mosaic/src/routing/__tests__/virtual-location.test.tsx index 3980d25db0f..df011edee47 100644 --- a/packages/mosaic/src/routing/__tests__/virtual-location.test.tsx +++ b/packages/mosaic/src/routing/__tests__/virtual-location.test.tsx @@ -2,15 +2,10 @@ import { act, renderHook } from '@testing-library/react'; import type { ReactNode } from 'react'; import { describe, expect, it } from 'vitest'; -import { - createVirtualLocation, - decodeParts, - encodeParts, - normalizeBase, - rawSegmentPartsOf, - urlFor, -} from '../virtual-location'; -import { useVirtualUrl, useVirtualUrlWrite, VirtualUrlProvider } from '../virtual-url'; +import { normalizeBase } from '../base-path'; +import type { SegmentParts } from '../location'; +import { createVirtualLocation } from '../virtual-location'; +import { useVirtualEntries, useVirtualWrite, VirtualStoreProvider } from '../virtual-store'; describe('normalizeBase', () => { it('strips surrounding slashes and collapses the root', () => { @@ -21,101 +16,86 @@ describe('normalizeBase', () => { }); }); -describe('rawSegmentPartsOf', () => { - it('reads the raw parts under the base', () => { - expect(rawSegmentPartsOf('/user/security', '/user')).toEqual(['security']); - expect(rawSegmentPartsOf('/user/billing/statement/st%2F1', '/user')).toEqual(['billing', 'statement', 'st%2F1']); - }); - - it('reads no parts at the base', () => { - expect(rawSegmentPartsOf('/user', '/user')).toEqual([]); - expect(rawSegmentPartsOf('/user/', '/user')).toEqual([]); - }); - - it('reads no parts outside the base', () => { - expect(rawSegmentPartsOf('/sign-in/factor-one', '/user')).toEqual([]); - expect(rawSegmentPartsOf('/users', '/user')).toEqual([]); - }); - - it('reads the whole path at the root base', () => { - expect(rawSegmentPartsOf('/security', '')).toEqual(['security']); - expect(rawSegmentPartsOf('/', '')).toEqual([]); - }); -}); - -describe('urlFor', () => { - it('joins the base and the raw parts', () => { - expect(urlFor('/user', ['security'])).toBe('/user/security'); - expect(urlFor('/user', [])).toBe('/user'); - expect(urlFor('', ['security'])).toBe('/security'); - expect(urlFor('', [])).toBe('/'); - }); -}); - -describe('decodeParts and encodeParts', () => { - it('round-trip any value', () => { - const parts = ['st/1', '100%', 'a b', 'ö', '']; - expect(decodeParts(encodeParts(parts))).toEqual(parts); - }); - - it('report no parts for a malformed escape', () => { - expect(decodeParts(['billing', '%E0%A4'])).toEqual([]); - }); -}); - -function wrapperAt(initialUrl: string) { +function wrapperWith(initial?: Record) { return ({ children }: { children: ReactNode }) => ( - {children} + {children} ); } function useLocationAt(basePath: string) { - const location = createVirtualLocation(basePath, useVirtualUrlWrite()); - return { location, parts: location.useSegmentParts(), url: useVirtualUrl() }; + const location = createVirtualLocation(basePath, useVirtualWrite()); + return { location, parts: location.useSegmentParts(), entries: useVirtualEntries() }; } describe('createVirtualLocation', () => { - it('derives decoded parts from the page virtual URL', () => { - const { result } = renderHook(() => useLocationAt('/user'), { wrapper: wrapperAt('/user/billing/st%2F1') }); - expect(result.current.parts).toEqual(['billing', 'st/1']); + it('reads its own entry of the store', () => { + const { result } = renderHook(() => useLocationAt('/user'), { + wrapper: wrapperWith({ '/user': ['security'], '/organization': ['members'] }), + }); + expect(result.current.parts).toEqual(['security']); }); - it('writes encoded parts under its base', () => { - const { result } = renderHook(() => useLocationAt('/user'), { wrapper: wrapperAt('/') }); + it('reads no parts when it has no entry', () => { + const { result } = renderHook(() => useLocationAt('/user'), { wrapper: wrapperWith() }); + expect(result.current.parts).toEqual([]); + }); + + it('writes its entry verbatim', () => { + const { result } = renderHook(() => useLocationAt('/user'), { wrapper: wrapperWith() }); act(() => { result.current.location.write(['billing', 'statement', 'st/1 %']); }); - expect(result.current.url).toBe('/user/billing/statement/st%2F1%20%25'); expect(result.current.parts).toEqual(['billing', 'statement', 'st/1 %']); + expect(result.current.entries.get('/user')).toEqual(['billing', 'statement', 'st/1 %']); + }); + + it('clears its entry', () => { + const { result } = renderHook(() => useLocationAt('/user'), { wrapper: wrapperWith({ '/user': ['security'] }) }); + + act(() => { + result.current.location.clear(); + }); + + expect(result.current.parts).toEqual([]); + expect(result.current.entries.has('/user')).toBe(false); }); - it('returns the same parts object while the URL is unchanged', () => { - const { result, rerender } = renderHook(() => useLocationAt('/user'), { wrapper: wrapperAt('/user/security') }); + it('returns the same parts object while its entry is unchanged', () => { + const { result, rerender } = renderHook(() => useLocationAt('/user'), { + wrapper: wrapperWith({ '/user': ['security'] }), + }); const first = result.current.parts; rerender(); expect(result.current.parts).toBe(first); }); - it('reports no parts for a malformed segment', () => { - const { result } = renderHook(() => useLocationAt('/user'), { wrapper: wrapperAt('/user/%E0%A4') }); - expect(result.current.parts).toEqual([]); - }); - - it('shares the page virtual URL between instances with different bases', () => { - const { result } = renderHook(() => [useLocationAt('/user'), useLocationAt('/sign-in')] as const, { - wrapper: wrapperAt('/user/security'), + it('keeps owners with different bases apart', () => { + const { result } = renderHook(() => [useLocationAt('/user'), useLocationAt('/organization')] as const, { + wrapper: wrapperWith({ '/user': ['security'] }), }); expect(result.current[0].parts).toEqual(['security']); expect(result.current[1].parts).toEqual([]); act(() => { - result.current[1].location.write(['factor-one']); + result.current[1].location.write(['members']); + }); + + expect(result.current[0].parts).toEqual(['security']); + expect(result.current[1].parts).toEqual(['members']); + + act(() => { + result.current[0].location.clear(); }); expect(result.current[0].parts).toEqual([]); - expect(result.current[1].parts).toEqual(['factor-one']); + expect(result.current[1].parts).toEqual(['members']); + }); + + it('treats base spellings alike', () => { + const { result } = renderHook(() => useLocationAt('user/'), { wrapper: wrapperWith({ '/user': ['security'] }) }); + expect(result.current.parts).toEqual(['security']); }); it('throws without a provider', () => { diff --git a/packages/mosaic/src/routing/base-path.ts b/packages/mosaic/src/routing/base-path.ts new file mode 100644 index 00000000000..6a516f87f02 --- /dev/null +++ b/packages/mosaic/src/routing/base-path.ts @@ -0,0 +1,5 @@ +// '/user/' -> '/user' 'user' -> '/user' '/' -> '' +export function normalizeBase(base: string): string { + const parts = base.split('/').filter(Boolean); + return parts.length ? `/${parts.join('/')}` : ''; +} diff --git a/packages/mosaic/src/routing/location.ts b/packages/mosaic/src/routing/location.ts index 9a6c9014b5b..29cca03a170 100644 --- a/packages/mosaic/src/routing/location.ts +++ b/packages/mosaic/src/routing/location.ts @@ -4,11 +4,15 @@ export interface WriteOptions { export type SegmentParts = readonly string[]; -// A location owns the encoding boundary. The parts it hands out are decoded exactly once, per part, -// as close to the source as the source allows; the parts it is given are encoded on write the same -// way. Nothing above a location encodes or decodes a route part again. `useSegmentParts` returns the -// same array for as long as the segment is unchanged, so callers can memoise on it. +// A location owns one segment: the entry for its base in whatever carries the page's routing state. +// It also owns the encoding boundary. The parts it hands out are decoded exactly once, per part, as +// close to the source as the source allows; the parts it is given are encoded on write the same way, +// where the carrier has an encoded form at all. Nothing above a location encodes or decodes a route +// part again. `useSegmentParts` returns the same array for as long as the segment is unchanged, so +// callers can memoise on it. `clear` gives the entry up, for an owner such as a modal that only +// holds it while open. export interface MosaicLocation { useSegmentParts: () => SegmentParts; write: (parts: SegmentParts, options?: WriteOptions) => void; + clear: () => void; } diff --git a/packages/mosaic/src/routing/routes.test-d.ts b/packages/mosaic/src/routing/routes.test-d.ts index f63b2f3e61f..44b874684f4 100644 --- a/packages/mosaic/src/routing/routes.test-d.ts +++ b/packages/mosaic/src/routing/routes.test-d.ts @@ -17,7 +17,7 @@ describe('RouteParams', () => { }); describe('Router', () => { - const location: MosaicLocation = { useSegmentParts: () => [], write: () => {} }; + const location: MosaicLocation = { useSegmentParts: () => [], write: () => {}, clear: () => {} }; const router = createRouter( { index: '', security: 'security', statement: 'billing/statement/:statementId' }, location, diff --git a/packages/mosaic/src/routing/use-mosaic-location.ts b/packages/mosaic/src/routing/use-mosaic-location.ts index 0df2ae80217..c720ebc5d83 100644 --- a/packages/mosaic/src/routing/use-mosaic-location.ts +++ b/packages/mosaic/src/routing/use-mosaic-location.ts @@ -2,7 +2,7 @@ import { useMemo } from 'react'; import type { MosaicLocation } from './location'; import { createVirtualLocation } from './virtual-location'; -import { useVirtualUrlWrite } from './virtual-url'; +import { useVirtualWrite } from './virtual-store'; export type MosaicLocationOptions = { routing: 'virtual'; path?: string }; @@ -14,7 +14,7 @@ export interface RoutingProps { } export function useMosaicLocation(options: MosaicLocationOptions): MosaicLocation { - const writeVirtualUrl = useVirtualUrlWrite(); + const write = useVirtualWrite(); const path = options.path ?? '/'; - return useMemo(() => createVirtualLocation(path, writeVirtualUrl), [path, writeVirtualUrl]); + return useMemo(() => createVirtualLocation(path, write), [path, write]); } diff --git a/packages/mosaic/src/routing/virtual-location.ts b/packages/mosaic/src/routing/virtual-location.ts index 9404b1a120d..42c41a4989f 100644 --- a/packages/mosaic/src/routing/virtual-location.ts +++ b/packages/mosaic/src/routing/virtual-location.ts @@ -1,57 +1,14 @@ -import { useMemo } from 'react'; +import { normalizeBase } from './base-path'; +import type { MosaicLocation } from './location'; +import { useVirtualSegment, type VirtualWrite } from './virtual-store'; -import type { MosaicLocation, SegmentParts } from './location'; -import { useVirtualUrl } from './virtual-url'; - -// The virtual URL is kept in its encoded form, like a real one, so all locations work the same - -export function normalizeBase(base: string): string { - const parts = base.split('/').filter(Boolean); - return parts.length ? `/${parts.join('/')}` : ''; -} - -function rawPartsOf(url: string): string[] { - return url.split('/').filter(Boolean); -} - -export function rawSegmentPartsOf(url: string, base: string): string[] { - const parts = rawPartsOf(url); - const baseParts = rawPartsOf(base); - if (baseParts.length > parts.length || baseParts.some((part, index) => parts[index] !== part)) { - return []; - } - return parts.slice(baseParts.length); -} - -export function urlFor(base: string, rawParts: readonly string[]): string { - return rawParts.length ? `${base}/${rawParts.join('/')}` : base || '/'; -} - -const EMPTY_PARTS: SegmentParts = Object.freeze([]); - -// A malformed escape means the segment cannot be read, so the location reports no parts rather -// than a half-decoded value that something downstream might decode again. -export function decodeParts(rawParts: readonly string[]): SegmentParts { - try { - return rawParts.map(part => decodeURIComponent(part)); - } catch { - return EMPTY_PARTS; - } -} - -export function encodeParts(parts: SegmentParts): string[] { - return parts.map(part => encodeURIComponent(part)); -} - -function useVirtualSegmentParts(base: string): SegmentParts { - const url = useVirtualUrl(); - return useMemo(() => decodeParts(rawSegmentPartsOf(url, base)), [url, base]); -} - -export function createVirtualLocation(basePath: string, write: (url: string) => void): MosaicLocation { +// One entry of the page's virtual store, read and written as decoded parts. Nothing to encode: +// virtual state has no URL form until a carrier projects it. +export function createVirtualLocation(basePath: string, write: VirtualWrite): MosaicLocation { const base = normalizeBase(basePath); return { - useSegmentParts: () => useVirtualSegmentParts(base), - write: parts => write(urlFor(base, encodeParts(parts))), + useSegmentParts: () => useVirtualSegment(base), + write: parts => write(base, parts), + clear: () => write(base, undefined), }; } diff --git a/packages/mosaic/src/routing/virtual-store.tsx b/packages/mosaic/src/routing/virtual-store.tsx new file mode 100644 index 00000000000..c4e874a2d76 --- /dev/null +++ b/packages/mosaic/src/routing/virtual-store.tsx @@ -0,0 +1,78 @@ +'use client'; + +import type { ReactNode } from 'react'; +import { createContext, useCallback, useContext, useState } from 'react'; + +import { normalizeBase } from './base-path'; +import type { SegmentParts } from './location'; + +/* + The page's virtual location + + We keep this in a centralized place instead of in each component so that it behaves the same as a + url in the other modes: it outlives the component, and it can be written to from the outside, by a + modal that opens a surface at a page or by a return after a redirect. + + For example, in hash mode a modal would have to remove itself from the hash on close. We want that + same behavior in virtual mode so each mode behaves identically and the components don't have to + reason about the differences. + + We only need the external behavior to match the other modes however, so internally we keep state + in a map instead of trying to cram it into a single "url"-like string. Nothing is encoded since it + never goes into a real url. +*/ + +export type VirtualEntries = ReadonlyMap; + +export type VirtualWrite = (base: string, parts: SegmentParts | undefined) => void; + +const EMPTY_PARTS: SegmentParts = Object.freeze([]); + +const VirtualEntriesContext = createContext(undefined); +const VirtualWriteContext = createContext(undefined); + +export interface VirtualStoreProviderProps { + initial?: Record; + children: ReactNode; +} + +export function VirtualStoreProvider({ initial, children }: VirtualStoreProviderProps) { + const [entries, setEntries] = useState( + () => new Map(Object.entries(initial ?? {}).map(([base, parts]) => [normalizeBase(base), parts])), + ); + const write = useCallback((base, parts) => { + setEntries(previous => { + const next = new Map(previous); + if (parts === undefined) { + next.delete(base); + } else { + next.set(base, parts); + } + return next; + }); + }, []); + + return ( + + {children} + + ); +} + +function missingProvider(): never { + throw new Error('[clerk] Virtual routing must be used within a .'); +} + +export function useVirtualEntries(): VirtualEntries { + return useContext(VirtualEntriesContext) ?? missingProvider(); +} + +export function useVirtualWrite(): VirtualWrite { + return useContext(VirtualWriteContext) ?? missingProvider(); +} + +// The entry for a base, or no parts. The array comes straight from the map, so it keeps its +// identity until that entry is written again. +export function useVirtualSegment(base: string): SegmentParts { + return useVirtualEntries().get(normalizeBase(base)) ?? EMPTY_PARTS; +} diff --git a/packages/mosaic/src/routing/virtual-url.tsx b/packages/mosaic/src/routing/virtual-url.tsx deleted file mode 100644 index 7ccc356bd39..00000000000 --- a/packages/mosaic/src/routing/virtual-url.tsx +++ /dev/null @@ -1,33 +0,0 @@ -'use client'; - -import type { ReactNode } from 'react'; -import { createContext, useContext, useState } from 'react'; - -const VirtualUrlValueContext = createContext(undefined); -const VirtualUrlWriteContext = createContext<((url: string) => void) | undefined>(undefined); - -export interface VirtualUrlProviderProps { - initialUrl?: string; - children: ReactNode; -} - -export function VirtualUrlProvider({ initialUrl = '/', children }: VirtualUrlProviderProps) { - const [url, setUrl] = useState(initialUrl); - return ( - - {children} - - ); -} - -function missingProvider(): never { - throw new Error('[clerk] Virtual routing must be used within a .'); -} - -export function useVirtualUrl(): string { - return useContext(VirtualUrlValueContext) ?? missingProvider(); -} - -export function useVirtualUrlWrite(): (url: string) => void { - return useContext(VirtualUrlWriteContext) ?? missingProvider(); -} From 4d8f83f28c3db9e2be3cef1ce1b92cf6d15038f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Wed, 4 Nov 2026 15:49:15 +0100 Subject: [PATCH 04/26] Add routing support to modal host Also configures separate paths for modals to separate them from the base components. --- .../organization-profile.modal-content.tsx | 2 + .../organization-profile.modal.ts | 2 + .../organization-profile.tsx | 13 ++- .../user-profile.modal-content.tsx | 7 +- .../user-profile/user-profile.modal.ts | 2 + .../features/user-profile/user-profile.tsx | 16 +++- packages/mosaic/src/modals/modal-host.tsx | 16 +++- packages/mosaic/src/modals/modal.types.ts | 6 ++ packages/mosaic/src/modals/use-modal.test.tsx | 86 +++++++++++++++++++ 9 files changed, 141 insertions(+), 9 deletions(-) diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.modal-content.tsx b/packages/mosaic/src/features/organization-profile/organization-profile.modal-content.tsx index b7ff553c1d7..098448890a9 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.modal-content.tsx +++ b/packages/mosaic/src/features/organization-profile/organization-profile.modal-content.tsx @@ -7,6 +7,7 @@ import type { OrganizationProfileModalContentProps } from './organization-profil export default function OrganizationProfileModalContent({ options, close, + path, }: OrganizationProfileModalContentProps): React.ReactElement { const { isLoaded, organization } = useOrganization(); const inactive = isLoaded && !organization; @@ -21,6 +22,7 @@ export default function OrganizationProfileModalContent({ return ( ); diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.modal.ts b/packages/mosaic/src/features/organization-profile/organization-profile.modal.ts index c9b211967fc..75e8c45d554 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.modal.ts +++ b/packages/mosaic/src/features/organization-profile/organization-profile.modal.ts @@ -5,4 +5,6 @@ export const organizationProfileModal = defineModalSurface import('./organization-profile.modal-content'), + path: '/modal/organization', + initialParts: options => options?.page?.split('/').filter(Boolean) ?? [], }); diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.tsx b/packages/mosaic/src/features/organization-profile/organization-profile.tsx index a0793018727..b71e1549693 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.tsx +++ b/packages/mosaic/src/features/organization-profile/organization-profile.tsx @@ -19,8 +19,8 @@ import { OrganizationProfileMembersPanel } from './organization-profile-members- /** Everything `` takes: your own pages and their order, the page it opens on, and where leaving the organization lands. */ export interface OrganizationProfileProps extends - Pick, - Pick { + Pick, + Pick { /** * The page the profile opens on: a built-in page's id, or a custom page's `path`. Unlike * `pageOrder`, it leaves the navigation order alone. A page the navigation does not list opens @@ -29,6 +29,12 @@ export interface OrganizationProfileProps * @default the first page in the navigation */ defaultPage?: OrganizationProfileViewProps['activePage']; + /** + * The base the profile's pages are nested under. Two profiles on one page need different bases. + * + * @default '/organization' + */ + path?: string; /** * Rendered while Clerk is loading. Nothing renders once the answer is that there is no active * organization. @@ -86,6 +92,7 @@ export function OrganizationProfile(props: OrganizationProfileProps) { customPages = organizationProfileProps?.customPages, pageOrder = organizationProfileProps?.pageOrder, defaultPage, + path = '/organization', afterLeaveOrganizationUrl = organizationProfileProps?.afterLeaveOrganizationUrl, fallback, apiKeysProps = organizationProfileProps?.apiKeysProps, @@ -95,7 +102,7 @@ export function OrganizationProfile(props: OrganizationProfileProps) { const { activePage, onPageChange } = useProfilePagesController( defaultPage ?? pageOrder?.[0] ?? 'general', pages && organizationProfilePageIds({ pages, customPages, pageOrder }), - { routing: 'virtual', path: '/organization' }, + { routing: 'virtual', path }, ); if (model.status === 'loading') { diff --git a/packages/mosaic/src/features/user-profile/user-profile.modal-content.tsx b/packages/mosaic/src/features/user-profile/user-profile.modal-content.tsx index c9d35d442de..32c8f1d907a 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.modal-content.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile.modal-content.tsx @@ -4,7 +4,11 @@ import React from 'react'; import { UserProfileSurface } from './user-profile'; import type { UserProfileModalContentProps } from './user-profile.modal.types'; -export default function UserProfileModalContent({ options, close }: UserProfileModalContentProps): React.ReactElement { +export default function UserProfileModalContent({ + options, + close, + path, +}: UserProfileModalContentProps): React.ReactElement { const { isLoaded, user } = useUser(); const signedOut = isLoaded && !user; @@ -19,6 +23,7 @@ export default function UserProfileModalContent({ options, close }: UserProfileM ); diff --git a/packages/mosaic/src/features/user-profile/user-profile.modal.ts b/packages/mosaic/src/features/user-profile/user-profile.modal.ts index 636d8aa504a..3f7fa7715d2 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.modal.ts +++ b/packages/mosaic/src/features/user-profile/user-profile.modal.ts @@ -5,4 +5,6 @@ export const userProfileModal = defineModalSurface({ id: 'userProfile', variant: 'profile', load: () => import('./user-profile.modal-content'), + path: '/modal/user', + initialParts: options => options?.page?.split('/').filter(Boolean) ?? [], }); diff --git a/packages/mosaic/src/features/user-profile/user-profile.tsx b/packages/mosaic/src/features/user-profile/user-profile.tsx index 9b82d833dc1..0b5869cc6e5 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile.tsx @@ -31,6 +31,12 @@ export interface UserProfileProps * @default the first page in the navigation */ defaultPage?: UserProfileViewProps['activePage']; + /** + * The base the profile's pages are nested under. Two profiles on one page need different bases. + * + * @default '/user' + */ + path?: string; /** * Rendered while Clerk is loading. Nothing renders once the answer is that nobody is signed in. */ @@ -157,6 +163,7 @@ export function UserProfileSurface({ customPages, pageOrder, defaultPage, + path = '/user', additionalOAuthScopes, fallback, apiKeysProps: { hide: hideAPIKeys = false, ...apiKeysProps } = {}, @@ -173,6 +180,7 @@ export function UserProfileSurface({ const viewProps: AccountViewProps = { elevation: 'flush', + path, initialPage: defaultPage ?? pageOrder?.[0] ?? 'account', customPages, pageOrder, @@ -203,12 +211,13 @@ export function UserProfileSurface({ type AccountViewProps = Omit & { initialPage: UserProfileViewProps['activePage']; + path: string; }; -function AccountUserProfileView({ initialPage, ...viewProps }: AccountViewProps) { +function AccountUserProfileView({ initialPage, path, ...viewProps }: AccountViewProps) { const { activePage, onPageChange } = useProfilePagesController(initialPage, userProfilePageIds(viewProps), { routing: 'virtual', - path: '/user', + path, }); return ( @@ -222,6 +231,7 @@ function AccountUserProfileView({ initialPage, ...viewProps }: AccountViewProps) function CombinedUserProfileView({ initialPage, + path, organizationProfileProps: { customPages, pageOrder, apiKeysProps, afterLeaveOrganizationUrl } = {}, ...viewProps }: AccountViewProps & Pick) { @@ -233,7 +243,7 @@ function CombinedUserProfileView({ const { activePage, onPageChange } = useProfilePagesController( initialPage, model.status === 'loading' ? undefined : userProfilePageIds({ ...viewProps, organization }), - { routing: 'virtual', path: '/user' }, + { routing: 'virtual', path }, ); return ( diff --git a/packages/mosaic/src/modals/modal-host.tsx b/packages/mosaic/src/modals/modal-host.tsx index b4fd3d901d9..3f05436ae4d 100644 --- a/packages/mosaic/src/modals/modal-host.tsx +++ b/packages/mosaic/src/modals/modal-host.tsx @@ -7,6 +7,7 @@ import { useMessages } from '../localization'; import { useActor } from '../machine/use-machine'; import type { PortalContainer } from '../primitives/portal'; import { PortalProvider } from '../primitives/portal'; +import { useMosaicLocation } from '../routing/use-mosaic-location'; import type { ModalHandle, ModalSurface, ModalSurfaces } from './modal.types'; export function ModalHost({ @@ -42,18 +43,28 @@ export function ModalSurfaceHost({ openCount: 0, }); + const routed = surface.path !== undefined; + const location = useMosaicLocation({ routing: 'virtual', path: surface.path ?? `/modal/${surface.id}` }); const modal = React.useMemo>(() => { const preload = () => surface.loader.send({ type: 'LOAD' }); return { open: (options, container) => { preload(); + if (routed) { + location.write(surface.initialParts?.(options) ?? [], { replace: true }); + } setState(previous => ({ open: true, options, container, openCount: previous.openCount + 1 })); }, - close: () => setState(previous => (previous.open ? { ...previous, open: false } : previous)), + close: () => { + if (routed && state.open) { + location.clear(); + } + setState(previous => (previous.open ? { ...previous, open: false } : previous)); + }, preload, isOpen: state.open, }; - }, [surface, state.open]); + }, [surface, routed, location, state.open]); return ( @@ -95,6 +106,7 @@ function ModalSlot({ key={state.openCount} options={state.options} close={close} + path={surface.path} /> ) : failed ? ( diff --git a/packages/mosaic/src/modals/modal.types.ts b/packages/mosaic/src/modals/modal.types.ts index b68d7d88c97..6a30d891bfe 100644 --- a/packages/mosaic/src/modals/modal.types.ts +++ b/packages/mosaic/src/modals/modal.types.ts @@ -2,17 +2,23 @@ import type { ComponentType, Context, ReactNode } from 'react'; import type { DialogVariant } from '../components/dialog/dialog'; import type { PortalContainer } from '../primitives/portal'; +import type { SegmentParts } from '../routing/location'; import type { ModalContentLoader } from './modal-content-loader'; export interface ModalContentProps { options: Options | undefined; close: () => void; + path: string | undefined; } export interface ModalSurfaceConfig { id: string; variant: DialogVariant; load: () => Promise<{ default: ComponentType> }>; + // If path is provided, the modal host writes it before opening and clears it on close + path?: string; + // Optional way to derive the initial segment to be written together with path on open + initialParts?: (options: Options | undefined) => SegmentParts; } export interface ModalHostSurface { diff --git a/packages/mosaic/src/modals/use-modal.test.tsx b/packages/mosaic/src/modals/use-modal.test.tsx index 609989e0188..58ddc8729ae 100644 --- a/packages/mosaic/src/modals/use-modal.test.tsx +++ b/packages/mosaic/src/modals/use-modal.test.tsx @@ -4,6 +4,7 @@ import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest'; import type { MosaicProviderProps } from '../mosaic-provider'; import { MosaicProvider } from '../mosaic-provider'; +import { useVirtualSegment } from '../routing/virtual-store'; import { defineModalSurface } from './define-modal-surface'; import type { ModalContentProps, ModalHandle, ModalSurface, ModalSurfaces } from './modal.types'; import { ModalHost } from './modal-host'; @@ -458,3 +459,88 @@ describe('useModal', () => { ).toThrow(/unknown.*MosaicProvider/); }); }); + +describe('a modal surface that routes', () => { + function Entry() { + return {JSON.stringify(useVirtualSegment('/modal/stub'))}; + } + + function routedSurface() { + return defineModalSurface({ + id: 'routed', + variant: 'card', + load: () => Promise.resolve({ default: StubContent }), + path: '/modal/stub', + initialParts: options => (options?.page ? options.page.split('/') : []), + }); + } + + function renderRouted(surface = routedSurface()) { + const ref: { handle: ModalHandle | undefined } = { handle: undefined }; + renderWithModals( + [surface], + <> + (ref.handle = h)} + /> + + , + ); + return ref; + } + + const entry = () => JSON.parse(screen.getByTestId('entry').textContent ?? 'null'); + + it('writes an empty entry under its base when opened with no page', async () => { + const ref = renderRouted(); + + act(() => ref.handle?.open()); + await flush(); + + expect(entry()).toEqual([]); + }); + + it('writes the page it was opened at', async () => { + const ref = renderRouted(); + + act(() => ref.handle?.open({ page: 'organization/general' })); + await flush(); + + expect(entry()).toEqual(['organization', 'general']); + }); + + it('clears its entry when closed, so it reopens fresh', async () => { + const ref = renderRouted(); + + act(() => ref.handle?.open({ page: 'security' })); + await flush(); + expect(entry()).toEqual(['security']); + + act(() => ref.handle?.close()); + await flush(); + expect(entry()).toEqual([]); + + act(() => ref.handle?.open()); + await flush(); + expect(entry()).toEqual([]); + }); + + it('hands its base to the content', async () => { + const surface = defineModalSurface({ + id: 'based', + variant: 'card', + load: () => + Promise.resolve({ + default: ({ path }: ModalContentProps) => {path}, + }), + path: '/modal/based', + }); + const ref = renderRouted(surface); + + act(() => ref.handle?.open()); + await flush(); + + expect(screen.getByTestId('base').textContent).toBe('/modal/based'); + }); +}); From 65a3f3fbcd7b9d255400cf995a84866eba0d5cde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Wed, 4 Nov 2026 16:05:08 +0100 Subject: [PATCH 05/26] Add loop protection for misconfigured duplicate paths --- .../src/routing/__tests__/use-router.test.tsx | 37 ++++++++++++++++++- packages/mosaic/src/routing/use-router.ts | 33 +++++++++++++---- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/packages/mosaic/src/routing/__tests__/use-router.test.tsx b/packages/mosaic/src/routing/__tests__/use-router.test.tsx index 895bbabd779..8e98fc95e76 100644 --- a/packages/mosaic/src/routing/__tests__/use-router.test.tsx +++ b/packages/mosaic/src/routing/__tests__/use-router.test.tsx @@ -1,6 +1,7 @@ +import { logger } from '@clerk/shared/logger'; import { act, renderHook } from '@testing-library/react'; import type { ReactNode } from 'react'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import type { SegmentParts } from '../location'; import type { RouteTable } from '../routes'; @@ -128,6 +129,40 @@ describe('useRouterScope', () => { expect(result.current.org.route).toBe('members'); }); + it('redirects again when a different unknown segment is written', () => { + const { result } = renderHook( + () => ({ router: useRouterScope(routes, userScope), segment: useUserSegment(), write: useVirtualWrite() }), + { wrapper: wrapperWith({ '/user': ['nowhere'] }) }, + ); + expect(result.current.segment).toEqual(['account']); + + act(() => { + result.current.write('/user', ['elsewhere']); + }); + expect(result.current.router.route).toBe('account'); + expect(result.current.segment).toEqual(['account']); + }); + + it('stops redirecting when two owners of one base keep writing each other back, and warns', () => { + const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); + const orgRoutes = { general: 'general', members: 'members' } as const; + const { result } = renderHook( + () => ({ + user: useRouterScope(routes, userScope), + org: useRouterScope(orgRoutes, { routing: 'virtual', path: '/user', notFound: 'general' }), + segment: useUserSegment(), + }), + { wrapper: wrapperWith() }, + ); + + expect(result.current.segment).toEqual(['general']); + expect(result.current.user.route).toBe('account'); + expect(result.current.org.route).toBe('general'); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toContain('"general" under "/user"'); + warn.mockRestore(); + }); + it('keeps the same result object between unrelated renders', () => { const { result, rerender } = renderHook(() => useRouterScope(routes, userScope), { wrapper: wrapperWith({ '/user': ['security'] }), diff --git a/packages/mosaic/src/routing/use-router.ts b/packages/mosaic/src/routing/use-router.ts index 66cf39b7722..e58b61d4970 100644 --- a/packages/mosaic/src/routing/use-router.ts +++ b/packages/mosaic/src/routing/use-router.ts @@ -1,6 +1,7 @@ -import { useEffect, useMemo, useState } from 'react'; +import { logger } from '@clerk/shared/logger'; +import { useEffect, useMemo, useRef, useState } from 'react'; -import type { MosaicLocation } from './location'; +import type { MosaicLocation, SegmentParts } from './location'; import type { Route, Router } from './router'; import { createRouter } from './router'; import type { RouteName, RouteTable } from './routes'; @@ -30,6 +31,10 @@ function useStableTable(routes: Routes): Routes { return useStableByKey(routes, Object.entries(routes).flat().join(KEY_SEPARATOR)); } +function sameParts(a: SegmentParts, b: SegmentParts): boolean { + return a.length === b.length && a.every((part, index) => part === b[index]); +} + function missingScope(): never { throw new Error('[clerk] useRouter needs a above it; the segment owner calls useRouterScope.'); } @@ -38,6 +43,7 @@ interface Routing { location: MosaicLocation; table: Routes; router: Router; + parts: SegmentParts; matched: Route | undefined; } @@ -47,7 +53,7 @@ function useRouting(routes: Routes, options: MosaicLo const router = useMemo(() => createRouter(table, location), [table, location]); const parts = location.useSegmentParts(); const matched = useMemo(() => router.parse(parts), [router, parts]); - return { location, table, router, matched }; + return { location, table, router, parts, matched }; } export interface RouterScopeOptions { @@ -72,18 +78,31 @@ export function useRouterScope( { routing, path, notFound }: RouterScopeOptions, ): RouterScope { const scope = useMemo(() => createRoutingScope({ routing, path }), [routing, path]); - const { location, table, router, matched } = useRouting(routes, { routing, path }); + const { location, table, router, parts, matched } = useRouting(routes, { routing, path }); const current = useMemo>( () => matched ?? { name: notFound, params: EMPTY_PARAMS }, [matched, notFound], ); const redirectTo = matched === undefined ? notFound : undefined; + const redirected = useRef<{ from: SegmentParts; left: boolean }>(undefined); useEffect(() => { - if (redirectTo !== undefined) { - location.write(routeParts(table, redirectTo), { replace: true }); + const last = redirected.current; + if (last && !sameParts(last.from, parts)) { + last.left = true; + } + if (redirectTo === undefined) { + return; + } + if (last?.left && sameParts(last.from, parts)) { + logger.warnOnce( + `[Clerk] The segment "${parts.join('/')}" under "${path ?? '/'}" was not found, but after updating the location to match the fallback page, something wrote back the segment again. Two routers probably share this base; give one of them a different path.`, + ); + return; } - }, [redirectTo, location, table]); + redirected.current = { from: parts, left: false }; + location.write(routeParts(table, redirectTo), { replace: true }); + }, [parts, redirectTo, location, table, path]); return useMemo( () => ({ route: current.name, params: current.params, go: router.go, scope }), From 94428ec5d9d8573b3eecb875ba5782bd1f83a552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Wed, 4 Nov 2026 16:32:02 +0100 Subject: [PATCH 06/26] Add animation test coverage --- packages/mosaic/src/modals/use-modal.test.tsx | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/packages/mosaic/src/modals/use-modal.test.tsx b/packages/mosaic/src/modals/use-modal.test.tsx index 58ddc8729ae..4240a0a16a7 100644 --- a/packages/mosaic/src/modals/use-modal.test.tsx +++ b/packages/mosaic/src/modals/use-modal.test.tsx @@ -1,9 +1,11 @@ +import { logger } from '@clerk/shared/logger'; import { act, cleanup, render, renderHook, screen, within } from '@testing-library/react'; import React from 'react'; import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest'; import type { MosaicProviderProps } from '../mosaic-provider'; import { MosaicProvider } from '../mosaic-provider'; +import { useRouterScope } from '../routing/use-router'; import { useVirtualSegment } from '../routing/virtual-store'; import { defineModalSurface } from './define-modal-surface'; import type { ModalContentProps, ModalHandle, ModalSurface, ModalSurfaces } from './modal.types'; @@ -526,6 +528,83 @@ describe('a modal surface that routes', () => { expect(entry()).toEqual([]); }); + describe('with content that routes', () => { + const pages = { account: 'account', security: 'security' } as const; + + function RoutingContent({ path }: ModalContentProps) { + const { route } = useRouterScope(pages, { routing: 'virtual', path, notFound: 'account' }); + return {route}; + } + + function routingSurface() { + return defineModalSurface({ + id: 'routing', + variant: 'card', + load: () => Promise.resolve({ default: RoutingContent }), + path: '/modal/stub', + initialParts: options => (options?.page ? options.page.split('/') : []), + }); + } + + function keepExitPending() { + const prototype = Element.prototype as { getAnimations?: unknown }; + const original = prototype.getAnimations; + prototype.getAnimations = () => [{ finished: new Promise(() => {}) }]; + onTestFinished(() => { + if (original) { + prototype.getAnimations = original; + } else { + delete prototype.getAnimations; + } + }); + } + + it('keeps the entry cleared after close, without the content redirecting it back', async () => { + const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); + const ref = renderRouted(routingSurface()); + + act(() => ref.handle?.open({ page: 'security' })); + await flush(); + expect(screen.getByTestId('route').textContent).toBe('security'); + + act(() => ref.handle?.close()); + await flush(); + expect(entry()).toEqual([]); + expect(warn).not.toHaveBeenCalled(); + }); + + it('keeps the entry cleared for the length of the exit transition', async () => { + keepExitPending(); + const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); + const ref = renderRouted(routingSurface()); + + act(() => ref.handle?.open({ page: 'security' })); + await flush(); + act(() => ref.handle?.close()); + await flush(); + + expect(screen.getByTestId('route')).toBeInTheDocument(); + expect(entry()).toEqual([]); + expect(warn).not.toHaveBeenCalled(); + }); + + it('opens at the requested page when reopened mid-exit', async () => { + keepExitPending(); + const ref = renderRouted(routingSurface()); + + act(() => ref.handle?.open()); + await flush(); + expect(entry()).toEqual(['account']); + act(() => ref.handle?.close()); + await flush(); + act(() => ref.handle?.open({ page: 'security' })); + await flush(); + + expect(entry()).toEqual(['security']); + expect(screen.getByTestId('route').textContent).toBe('security'); + }); + }); + it('hands its base to the content', async () => { const surface = defineModalSurface({ id: 'based', From a6c22a1682188bc478a9794d08851593776ddee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Thu, 5 Nov 2026 09:22:40 +0100 Subject: [PATCH 07/26] Reject unroutable custom pages and add second infinite redirect guard --- .../organization-profile.layout.ts | 4 ++-- .../profile-pages/profile-pages.layout.ts | 14 ++++++++++++++ .../__tests__/user-profile.layout.test.ts | 17 ++++++++++++++++- .../user-profile/user-profile.layout.ts | 4 ++-- packages/mosaic/src/modals/modal-host.tsx | 2 +- .../src/routing/__tests__/routes.test.ts | 18 +++++++++++++++++- .../src/routing/__tests__/use-router.test.tsx | 17 +++++++++++++++++ packages/mosaic/src/routing/routes.ts | 7 +++++++ packages/mosaic/src/routing/use-router.ts | 15 +++++++++++++-- 9 files changed, 89 insertions(+), 9 deletions(-) diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.layout.ts b/packages/mosaic/src/features/organization-profile/organization-profile.layout.ts index 176d7bd842c..1c910312772 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.layout.ts +++ b/packages/mosaic/src/features/organization-profile/organization-profile.layout.ts @@ -5,7 +5,7 @@ import { Key } from '../../icons/glyphs/key'; import { SecurityLockSquare } from '../../icons/glyphs/security-lock-square'; import { Users } from '../../icons/glyphs/users'; import { applyOrder } from '../../utils/apply-order'; -import { withUniquePaths } from '../profile-pages/profile-pages.layout'; +import { withRoutablePaths, withUniquePaths } from '../profile-pages/profile-pages.layout'; import type { CustomOrganizationProfilePage, OrganizationProfilePageId, @@ -45,7 +45,7 @@ export function resolveOrganizationProfilePages( customPages: readonly CustomOrganizationProfilePage[] = [], order?: readonly string[], ): OrganizationProfileNavEntry[] { - const uniqueCustomPages = withUniquePaths(customPages); + const uniqueCustomPages = withUniquePaths(withRoutablePaths(customPages)); const customById = new Map(uniqueCustomPages.map(page => [page.path, page])); const builtInIds = new Set(builtIn); const entries: OrganizationProfileNavEntry[] = [ diff --git a/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts b/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts index 96e8f6b5809..4c733452c17 100644 --- a/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts +++ b/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts @@ -1,5 +1,19 @@ import { logger } from '@clerk/shared/logger'; +import { isLiteralPattern } from '../../routing/routes'; + +export function withRoutablePaths(pages: readonly Page[]): Page[] { + return pages.filter(page => { + if (isLiteralPattern(page.path)) { + return true; + } + logger.warnOnce( + `[Clerk] Custom page path "${page.path}" is not a plain URL segment, so the page is left out. Use URL-safe characters only, without a leading colon.`, + ); + return false; + }); +} + export function withUniquePaths(pages: readonly Page[]): Page[] { const seen = new Set(); return pages.filter(page => { diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts b/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts index 0e3b207b2ed..462b251e5c9 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts @@ -1,4 +1,5 @@ -import { describe, expect, it } from 'vitest'; +import { logger } from '@clerk/shared/logger'; +import { describe, expect, it, vi } from 'vitest'; import { getAvailableUserProfilePages, resolveUserProfilePages } from '../user-profile.layout'; import type { CustomProfilePage, UserProfilePages } from '../user-profile.types'; @@ -36,6 +37,20 @@ describe('resolveUserProfilePages', () => { ]); }); + it('leaves out a custom page whose path cannot be a route, and warns', () => { + const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); + const spaced: CustomProfilePage = { label: 'My page', path: 'my page', content: null }; + const param: CustomProfilePage = { label: 'Param', path: ':id', content: null }; + + expect(resolveUserProfilePages(['account'], [spaced, terms, param])).toEqual([ + { id: 'account' }, + { id: 'terms', custom: terms }, + ]); + expect(warn).toHaveBeenCalledTimes(2); + expect(warn.mock.calls[0][0]).toContain('"my page"'); + warn.mockRestore(); + }); + it('lists the built-ins, then the custom pages by path', () => { expect(resolveUserProfilePages(['account', 'security'], [terms, help])).toEqual([ { id: 'account' }, diff --git a/packages/mosaic/src/features/user-profile/user-profile.layout.ts b/packages/mosaic/src/features/user-profile/user-profile.layout.ts index e3df4d774f2..fa625790efe 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.layout.ts +++ b/packages/mosaic/src/features/user-profile/user-profile.layout.ts @@ -8,7 +8,7 @@ import { UserCircle } from '../../icons/glyphs/user-circle'; import { applyOrder } from '../../utils/apply-order'; import { organizationProfilePageIds } from '../organization-profile/organization-profile.layout'; import type { OrganizationProfilePageListOptions } from '../organization-profile/organization-profile.types'; -import { withUniquePaths } from '../profile-pages/profile-pages.layout'; +import { withRoutablePaths, withUniquePaths } from '../profile-pages/profile-pages.layout'; import type { CustomProfilePage, UserProfilePageId, UserProfilePages } from './user-profile.types'; /** The built-in pages in the order the profile lists them before a consumer reorders anything. */ @@ -47,7 +47,7 @@ export function resolveUserProfilePages( customPages: readonly CustomProfilePage[] = [], order?: readonly string[], ): UserProfileNavEntry[] { - const uniqueCustomPages = withUniquePaths(customPages); + const uniqueCustomPages = withUniquePaths(withRoutablePaths(customPages)); const customById = new Map(uniqueCustomPages.map(page => [page.path, page])); const builtInIds = new Set(builtIn); const entries: UserProfileNavEntry[] = [ diff --git a/packages/mosaic/src/modals/modal-host.tsx b/packages/mosaic/src/modals/modal-host.tsx index 3f05436ae4d..fdb724ac938 100644 --- a/packages/mosaic/src/modals/modal-host.tsx +++ b/packages/mosaic/src/modals/modal-host.tsx @@ -51,7 +51,7 @@ export function ModalSurfaceHost({ open: (options, container) => { preload(); if (routed) { - location.write(surface.initialParts?.(options) ?? [], { replace: true }); + location.write(surface.initialParts?.(options) ?? []); } setState(previous => ({ open: true, options, container, openCount: previous.openCount + 1 })); }, diff --git a/packages/mosaic/src/routing/__tests__/routes.test.ts b/packages/mosaic/src/routing/__tests__/routes.test.ts index 18233a11b86..53da99d423f 100644 --- a/packages/mosaic/src/routing/__tests__/routes.test.ts +++ b/packages/mosaic/src/routing/__tests__/routes.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { buildParts, compilePattern, compileRoutes, matchRoute, routeParts } from '../routes'; +import { buildParts, compilePattern, compileRoutes, isLiteralPattern, matchRoute, routeParts } from '../routes'; const routes = compileRoutes({ index: '', @@ -72,6 +72,22 @@ describe('compilePattern', () => { }); }); +describe('isLiteralPattern', () => { + it('accepts plain parts', () => { + expect(isLiteralPattern('api-keys')).toBe(true); + expect(isLiteralPattern('organization/general')).toBe(true); + }); + + it('rejects an empty pattern, a param, and a literal that encoding would change', () => { + expect(isLiteralPattern('')).toBe(false); + expect(isLiteralPattern('/')).toBe(false); + expect(isLiteralPattern(':id')).toBe(false); + expect(isLiteralPattern('terms/:id?')).toBe(false); + expect(isLiteralPattern('my page')).toBe(false); + expect(isLiteralPattern('my%20page')).toBe(false); + }); +}); + describe('buildParts', () => { it('returns a static pattern unchanged', () => { expect(buildParts(compilePattern('security'), {})).toEqual(['security']); diff --git a/packages/mosaic/src/routing/__tests__/use-router.test.tsx b/packages/mosaic/src/routing/__tests__/use-router.test.tsx index 8e98fc95e76..f9ab01be686 100644 --- a/packages/mosaic/src/routing/__tests__/use-router.test.tsx +++ b/packages/mosaic/src/routing/__tests__/use-router.test.tsx @@ -143,6 +143,23 @@ describe('useRouterScope', () => { expect(result.current.segment).toEqual(['account']); }); + it('does not redirect to a not-found route that cannot match on its own, and warns', () => { + const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); + const { result } = renderHook( + () => ({ + router: useRouterScope(routes, { routing: 'virtual', path: '/user', notFound: 'statement' }), + segment: useUserSegment(), + }), + { wrapper: wrapperWith({ '/user': ['nowhere'] }) }, + ); + + expect(result.current.router.route).toBe('statement'); + expect(result.current.segment).toEqual(['nowhere']); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toContain('"statement" under "/user"'); + warn.mockRestore(); + }); + it('stops redirecting when two owners of one base keep writing each other back, and warns', () => { const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); const orgRoutes = { general: 'general', members: 'members' } as const; diff --git a/packages/mosaic/src/routing/routes.ts b/packages/mosaic/src/routing/routes.ts index 5d55d71ab12..5f10ecbacd3 100644 --- a/packages/mosaic/src/routing/routes.ts +++ b/packages/mosaic/src/routing/routes.ts @@ -57,6 +57,13 @@ function assertPlainLiteral(part: string): void { } } +// 'api-keys' -> true 'my page' -> false ':id' -> false '' -> false +// Whether a string from outside, such as a custom page path, can be used as a literal route as is. +export function isLiteralPattern(pattern: string): boolean { + const parts = partsOf(pattern); + return parts.length > 0 && parts.every(part => !part.startsWith(':') && encodeURIComponent(part) === part); +} + // 'billing/plans/:planId?' -> [{ param: false, value: 'billing' }, { param: false, value: 'plans' }, { param: true, name: 'planId', optional: true }] export function compilePattern(pattern: string): PatternPart[] { return partsOf(pattern).map(part => { diff --git a/packages/mosaic/src/routing/use-router.ts b/packages/mosaic/src/routing/use-router.ts index e58b61d4970..862300107b7 100644 --- a/packages/mosaic/src/routing/use-router.ts +++ b/packages/mosaic/src/routing/use-router.ts @@ -84,6 +84,11 @@ export function useRouterScope( [matched, notFound], ); + const fallback = useMemo(() => { + const target = routeParts(table, notFound); + return router.parse(target) ? target : undefined; + }, [table, router, notFound]); + const redirectTo = matched === undefined ? notFound : undefined; const redirected = useRef<{ from: SegmentParts; left: boolean }>(undefined); useEffect(() => { @@ -94,6 +99,12 @@ export function useRouterScope( if (redirectTo === undefined) { return; } + if (fallback === undefined) { + logger.warnOnce( + `[Clerk] The notFound route "${redirectTo}" under "${path ?? '/'}" is either missing or needs params, so it cannot be a fallback. Use a route without params.`, + ); + return; + } if (last?.left && sameParts(last.from, parts)) { logger.warnOnce( `[Clerk] The segment "${parts.join('/')}" under "${path ?? '/'}" was not found, but after updating the location to match the fallback page, something wrote back the segment again. Two routers probably share this base; give one of them a different path.`, @@ -101,8 +112,8 @@ export function useRouterScope( return; } redirected.current = { from: parts, left: false }; - location.write(routeParts(table, redirectTo), { replace: true }); - }, [parts, redirectTo, location, table, path]); + location.write(fallback, { replace: true }); + }, [parts, redirectTo, fallback, location, path]); return useMemo( () => ({ route: current.name, params: current.params, go: router.go, scope }), From e80133119b085d01ade582f3709ba14ed37fa567 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Thu, 5 Nov 2026 09:35:50 +0100 Subject: [PATCH 08/26] Refactor the infinite redirect detection --- .../src/routing/__tests__/use-router.test.tsx | 33 +++++++++++++++---- packages/mosaic/src/routing/use-router.ts | 15 ++++----- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/packages/mosaic/src/routing/__tests__/use-router.test.tsx b/packages/mosaic/src/routing/__tests__/use-router.test.tsx index f9ab01be686..31fbca7b86d 100644 --- a/packages/mosaic/src/routing/__tests__/use-router.test.tsx +++ b/packages/mosaic/src/routing/__tests__/use-router.test.tsx @@ -1,7 +1,7 @@ import { logger } from '@clerk/shared/logger'; -import { act, renderHook } from '@testing-library/react'; +import { act, configure, renderHook } from '@testing-library/react'; import type { ReactNode } from 'react'; -import { describe, expect, it, vi } from 'vitest'; +import { describe, expect, it, onTestFinished, vi } from 'vitest'; import type { SegmentParts } from '../location'; import type { RouteTable } from '../routes'; @@ -129,18 +129,37 @@ describe('useRouterScope', () => { expect(result.current.org.route).toBe('members'); }); - it('redirects again when a different unknown segment is written', () => { + it('redirects every time an unknown segment is written, the same one included', () => { + const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); const { result } = renderHook( () => ({ router: useRouterScope(routes, userScope), segment: useUserSegment(), write: useVirtualWrite() }), { wrapper: wrapperWith({ '/user': ['nowhere'] }) }, ); expect(result.current.segment).toEqual(['account']); - act(() => { - result.current.write('/user', ['elsewhere']); + for (const unknown of ['elsewhere', 'nowhere', 'nowhere']) { + act(() => { + result.current.write('/user', [unknown]); + }); + expect(result.current.router.route).toBe('account'); + expect(result.current.segment).toEqual(['account']); + } + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + it('redirects under StrictMode without mistaking its own repeated effect for a collision', () => { + configure({ reactStrictMode: true }); + onTestFinished(() => configure({ reactStrictMode: false })); + const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); + const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), segment: useUserSegment() }), { + wrapper: wrapperWith({ '/user': ['nowhere'] }), }); + expect(result.current.router.route).toBe('account'); expect(result.current.segment).toEqual(['account']); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); }); it('does not redirect to a not-found route that cannot match on its own, and warns', () => { @@ -160,7 +179,7 @@ describe('useRouterScope', () => { warn.mockRestore(); }); - it('stops redirecting when two owners of one base keep writing each other back, and warns', () => { + it('stops redirecting when another owner of the same base overwrites its redirect, and warns', () => { const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); const orgRoutes = { general: 'general', members: 'members' } as const; const { result } = renderHook( @@ -176,7 +195,7 @@ describe('useRouterScope', () => { expect(result.current.user.route).toBe('account'); expect(result.current.org.route).toBe('general'); expect(warn).toHaveBeenCalledTimes(1); - expect(warn.mock.calls[0][0]).toContain('"general" under "/user"'); + expect(warn.mock.calls[0][0]).toContain('wrote "general" instead'); warn.mockRestore(); }); diff --git a/packages/mosaic/src/routing/use-router.ts b/packages/mosaic/src/routing/use-router.ts index 862300107b7..0d8a0157198 100644 --- a/packages/mosaic/src/routing/use-router.ts +++ b/packages/mosaic/src/routing/use-router.ts @@ -90,12 +90,10 @@ export function useRouterScope( }, [table, router, notFound]); const redirectTo = matched === undefined ? notFound : undefined; - const redirected = useRef<{ from: SegmentParts; left: boolean }>(undefined); + const redirect = useRef<{ from: SegmentParts; to: SegmentParts }>(undefined); useEffect(() => { - const last = redirected.current; - if (last && !sameParts(last.from, parts)) { - last.left = true; - } + const last = redirect.current; + redirect.current = undefined; if (redirectTo === undefined) { return; } @@ -105,13 +103,14 @@ export function useRouterScope( ); return; } - if (last?.left && sameParts(last.from, parts)) { + if (last && !sameParts(last.from, parts) && !sameParts(last.to, parts)) { + // This breaks what could otherwise be an infinite redirect loop logger.warnOnce( - `[Clerk] The segment "${parts.join('/')}" under "${path ?? '/'}" was not found, but after updating the location to match the fallback page, something wrote back the segment again. Two routers probably share this base; give one of them a different path.`, + `[Clerk] The segment "${last.from.join('/')}" under "${path ?? '/'}" was not found, but after updating the location to match the fallback page, something wrote "${parts.join('/')}" instead. Two routers probably share this base; give one of them a different path.`, ); return; } - redirected.current = { from: parts, left: false }; + redirect.current = { from: parts, to: fallback }; location.write(fallback, { replace: true }); }, [parts, redirectTo, fallback, location, path]); From 408c25c383d209daa419ee57b286012464fd14b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Thu, 5 Nov 2026 09:45:13 +0100 Subject: [PATCH 09/26] Add more tests and configure lint rules --- .../__tests__/user-profile.feature.test.tsx | 48 +++++++++++++++++++ packages/mosaic/src/modals/use-modal.test.tsx | 11 ++++- packages/swingset/tsconfig.json | 23 ++++++--- 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx index 7fbbdb778f2..863a326167b 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx @@ -130,6 +130,54 @@ describe('UserProfile', () => { expect(screen.getByRole('tab', { name: 'Account' })).toHaveAttribute('aria-selected', 'true'); }); + it('keeps its page when remounted under the same provider', async () => { + serve(); + const { rerender } = await renderWithClerk(); + const user = userEvent.setup(); + + await user.click(screen.getByRole('tab', { name: 'Security' })); + rerender(); + + expect(screen.getByRole('tab', { name: 'Security' })).toHaveAttribute('aria-selected', 'true'); + expect(screen.getByRole('heading', { level: 3, name: 'Security' })).toBeVisible(); + }); + + it('falls back to the default page when the current page disappears', async () => { + serve({ userAPIKeysEnabled: true }); + const { rerender } = await renderWithClerk(); + const user = userEvent.setup(); + + await user.click(screen.getByRole('tab', { name: 'API Keys' })); + expect(screen.getByRole('tab', { name: 'API Keys' })).toHaveAttribute('aria-selected', 'true'); + + rerender( + , + ); + + await waitFor(() => expect(screen.getByRole('tab', { name: 'Security' })).toHaveAttribute('aria-selected', 'true')); + expect(tabNames()).toEqual(['Account', 'Security']); + }); + + it('routes two profiles on one page independently when they have different bases', async () => { + serve(); + await renderWithClerk( + <> + + + , + ); + const user = userEvent.setup(); + const [first, second] = screen.getAllByRole('navigation', { name: 'User profile' }).map(nav => within(nav)); + + await user.click(first.getByRole('tab', { name: 'Security' })); + + expect(first.getByRole('tab', { name: 'Security' })).toHaveAttribute('aria-selected', 'true'); + expect(second.getByRole('tab', { name: 'Account' })).toHaveAttribute('aria-selected', 'true'); + }); + it('passes additional OAuth scopes to connected accounts', async () => { serveFapi(signedIn()); await renderWithClerk(); diff --git a/packages/mosaic/src/modals/use-modal.test.tsx b/packages/mosaic/src/modals/use-modal.test.tsx index 4240a0a16a7..f3ee968e516 100644 --- a/packages/mosaic/src/modals/use-modal.test.tsx +++ b/packages/mosaic/src/modals/use-modal.test.tsx @@ -1,5 +1,5 @@ import { logger } from '@clerk/shared/logger'; -import { act, cleanup, render, renderHook, screen, within } from '@testing-library/react'; +import { act, cleanup, configure, render, renderHook, screen, within } from '@testing-library/react'; import React from 'react'; import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest'; @@ -559,7 +559,14 @@ describe('a modal surface that routes', () => { }); } - it('keeps the entry cleared after close, without the content redirecting it back', async () => { + it.each([ + { name: 'normally', strict: false }, + { name: 'in StrictMode', strict: true }, + ])('keeps the entry cleared after close, without the content redirecting it back, $name', async ({ strict }) => { + if (strict) { + configure({ reactStrictMode: true }); + onTestFinished(() => configure({ reactStrictMode: false })); + } const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); const ref = renderRouted(routingSurface()); diff --git a/packages/swingset/tsconfig.json b/packages/swingset/tsconfig.json index ebb3a20eb3a..643897f3f4c 100644 --- a/packages/swingset/tsconfig.json +++ b/packages/swingset/tsconfig.json @@ -3,16 +3,25 @@ "target": "ES2022", "module": "preserve", "moduleResolution": "bundler", - "jsx": "react-jsx", + "jsx": "preserve", "strict": true, "skipLibCheck": true, "verbatimModuleSyntax": true, "moduleDetection": "force", - "lib": ["ES2023", "DOM"], + "lib": [ + "ES2023", + "DOM" + ], "paths": { - "@/*": ["./src/*"], - "@clerk/mosaic": ["../mosaic/src"], - "@clerk/mosaic/*": ["../mosaic/src/*"] + "@/*": [ + "./src/*" + ], + "@clerk/mosaic": [ + "../mosaic/src" + ], + "@clerk/mosaic/*": [ + "../mosaic/src/*" + ] }, "plugins": [ { @@ -32,5 +41,7 @@ ".next/types/**/*.ts", ".next/dev/types/**/*.ts" ], - "exclude": ["node_modules"] + "exclude": [ + "node_modules" + ] } From a862b8fbb4105e2a1abffe0f09d8945b6133cc51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Thu, 5 Nov 2026 10:42:43 +0100 Subject: [PATCH 10/26] Remove defaultPage Also makes some tweaks around how to resolve initial modal routes. --- .../organization-profile.feature.test.tsx | 25 +---- .../organization-profile.modal-content.tsx | 3 +- .../organization-profile.modal.ts | 3 +- .../organization-profile.modal.types.ts | 3 +- .../organization-profile.tsx | 18 +--- .../__tests__/profile-pages.layout.test.ts | 26 +++++ .../profile-pages/profile-pages.controller.ts | 20 ++-- .../profile-pages/profile-pages.layout.ts | 13 +++ .../__tests__/user-profile.feature.test.tsx | 95 ++++++++++++------- .../user-profile.modal-content.tsx | 3 +- .../user-profile/user-profile.modal.ts | 3 +- .../user-profile/user-profile.modal.types.ts | 3 +- .../features/user-profile/user-profile.tsx | 32 ++----- 13 files changed, 130 insertions(+), 117 deletions(-) create mode 100644 packages/mosaic/src/features/profile-pages/__tests__/profile-pages.layout.test.ts diff --git a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.feature.test.tsx b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.feature.test.tsx index f52f2cadd02..a5075be1efc 100644 --- a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.feature.test.tsx +++ b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.feature.test.tsx @@ -108,14 +108,10 @@ describe('OrganizationProfile', () => { it('passes the API keys options to the API keys page', async () => { serve({ orgAPIKeysEnabled: true, permissions: ['org:sys_api_keys:read', 'org:sys_api_keys:manage'] }); - await renderWithClerk( - , - ); + await renderWithClerk(); const user = userEvent.setup(); + await user.click(profileNav().getByRole('tab', { name: 'API Keys' })); await screen.findByText('API keys loaded'); const dialog = await openDialog(user, screen.getByRole('button', { name: 'Create API key' }), { name: 'Add new API key', @@ -149,20 +145,6 @@ describe('OrganizationProfile', () => { expect(screen.getByText('Usage this month')).toBeVisible(); }); - it('opens on the default page without reordering the navigation', async () => { - serve(); - await renderWithClerk(); - expect(tabNames()).toEqual(['General', 'Members']); - expect(profileNav().getByRole('tab', { name: 'Members' })).toHaveAttribute('aria-selected', 'true'); - expect(screen.getByRole('heading', { level: 3, name: 'Members' })).toBeVisible(); - }); - - it('opens on the first page when the default page is unavailable', async () => { - serve({ orgAPIKeysEnabled: false }); - await renderWithClerk(); - expect(profileNav().getByRole('tab', { name: 'General' })).toHaveAttribute('aria-selected', 'true'); - }); - it('sends the user to afterLeaveOrganizationUrl after leaving from the general page', async () => { const fapi = serve(); const { navigate } = await renderWithClerk(); @@ -190,7 +172,8 @@ describe('OrganizationProfile', () => { client: fapiClient([fapiSession({ id: 'sess_1', user, last_active_organization_id: acme.id })]), memberships, }); - const { clerk } = await renderWithClerk(); + const { clerk } = await renderWithClerk(); + await userEvent.setup().click(profileNav().getByRole('tab', { name: 'Members' })); await act(() => clerk.setActive({ organization: globex.id })); await waitFor(() => expect(tabNames()).toEqual(['General'])); diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.modal-content.tsx b/packages/mosaic/src/features/organization-profile/organization-profile.modal-content.tsx index 098448890a9..cfe672b6d37 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.modal-content.tsx +++ b/packages/mosaic/src/features/organization-profile/organization-profile.modal-content.tsx @@ -18,12 +18,11 @@ export default function OrganizationProfileModalContent({ } }, [inactive, close]); - const { page, ...props } = options ?? {}; + const { page: _page, ...props } = options ?? {}; return ( ); } diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.modal.ts b/packages/mosaic/src/features/organization-profile/organization-profile.modal.ts index 75e8c45d554..20d5bd1f978 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.modal.ts +++ b/packages/mosaic/src/features/organization-profile/organization-profile.modal.ts @@ -1,4 +1,5 @@ import { defineModalSurface } from '../../modals/define-modal-surface'; +import { pageParts } from '../profile-pages/profile-pages.layout'; import type { OrganizationProfileModalOptions } from './organization-profile.modal.types'; export const organizationProfileModal = defineModalSurface({ @@ -6,5 +7,5 @@ export const organizationProfileModal = defineModalSurface import('./organization-profile.modal-content'), path: '/modal/organization', - initialParts: options => options?.page?.split('/').filter(Boolean) ?? [], + initialParts: options => pageParts(options?.page), }); diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.modal.types.ts b/packages/mosaic/src/features/organization-profile/organization-profile.modal.types.ts index 95351810de1..36f6fc4cf9e 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.modal.types.ts +++ b/packages/mosaic/src/features/organization-profile/organization-profile.modal.types.ts @@ -1,11 +1,12 @@ import type { ModalContentProps } from '../../modals/modal.types'; import type { OrganizationProfileProps } from './organization-profile'; +import type { OrganizationProfileViewProps } from './organization-profile.view'; export interface OrganizationProfileModalOptions extends Pick< OrganizationProfileProps, 'customPages' | 'pageOrder' | 'afterLeaveOrganizationUrl' | 'apiKeysProps' > { - page?: OrganizationProfileProps['defaultPage']; + page?: OrganizationProfileViewProps['activePage']; } export type OrganizationProfileModalContentProps = ModalContentProps; diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.tsx b/packages/mosaic/src/features/organization-profile/organization-profile.tsx index b71e1549693..402590bfbbb 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.tsx +++ b/packages/mosaic/src/features/organization-profile/organization-profile.tsx @@ -21,14 +21,6 @@ export interface OrganizationProfileProps extends Pick, Pick { - /** - * The page the profile opens on: a built-in page's id, or a custom page's `path`. Unlike - * `pageOrder`, it leaves the navigation order alone. A page the navigation does not list opens - * the first page instead. - * - * @default the first page in the navigation - */ - defaultPage?: OrganizationProfileViewProps['activePage']; /** * The base the profile's pages are nested under. Two profiles on one page need different bases. * @@ -66,12 +58,6 @@ export interface OrganizationProfileProps * ``` * * @example - * `defaultPage` picks the page the profile opens on without moving it in the navigation. - * ```tsx - * - * ``` - * - * @example * `customPages` adds your own pages; `pageOrder` places them among the built-in ones. * ```tsx * { + it('maps the built-in ids to the segments legacy used', () => { + expect(pageSegment('apiKeys')).toBe('api-keys'); + expect(pageSegment('organization/apiKeys')).toBe('organization/api-keys'); + expect(pageSegment('security')).toBe('security'); + expect(pageSegment('organization/general')).toBe('organization/general'); + }); + + it('leaves custom page paths as they are', () => { + expect(pageSegment('usage')).toBe('usage'); + expect(pageSegment('docs/apiKeys')).toBe('docs/apiKeys'); + }); +}); + +describe('pageParts', () => { + it('splits a page id into the parts of its segment', () => { + expect(pageParts(undefined)).toEqual([]); + expect(pageParts('apiKeys')).toEqual(['api-keys']); + expect(pageParts('organization/general')).toEqual(['organization', 'general']); + expect(pageParts('/usage/')).toEqual(['usage']); + }); +}); diff --git a/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts b/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts index e886a727980..2382e8f5740 100644 --- a/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts +++ b/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts @@ -1,17 +1,15 @@ import type { MosaicLocationOptions } from '../../routing/use-mosaic-location'; import { useRouterScope } from '../../routing/use-router'; +import { pageSegment } from './profile-pages.layout'; -export function useProfilePagesController( - initialPage: string, - pageIds: readonly string[] | undefined, - routing: MosaicLocationOptions, -) { - const firstPage = pageIds?.[0]; - const fallbackPage = pageIds && firstPage !== undefined && !pageIds.includes(initialPage) ? firstPage : initialPage; - const { route, go } = useRouterScope(Object.fromEntries((pageIds ?? [initialPage]).map(id => [id, id])), { - ...routing, - notFound: fallbackPage, - }); +const NO_PAGE = ''; + +export function useProfilePagesController(pageIds: readonly string[], routing: MosaicLocationOptions) { + const rootPage = pageIds[0] ?? NO_PAGE; + const { route, go } = useRouterScope( + Object.fromEntries(pageIds.map(id => [id, id === rootPage ? '' : pageSegment(id)])), + { ...routing, notFound: rootPage }, + ); return { activePage: route, onPageChange: go }; } diff --git a/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts b/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts index 4c733452c17..1338c947eeb 100644 --- a/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts +++ b/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts @@ -2,6 +2,19 @@ import { logger } from '@clerk/shared/logger'; import { isLiteralPattern } from '../../routing/routes'; +const BUILT_IN_SEGMENTS: Readonly> = { + apiKeys: 'api-keys', + 'organization/apiKeys': 'organization/api-keys', +}; + +export function pageSegment(id: string): string { + return BUILT_IN_SEGMENTS[id] ?? id; +} + +export function pageParts(page: string | undefined): string[] { + return page === undefined ? [] : pageSegment(page).split('/').filter(Boolean); +} + export function withRoutablePaths(pages: readonly Page[]): Page[] { return pages.filter(page => { if (isLiteralPattern(page.path)) { diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx index 863a326167b..bdf0c948072 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx @@ -15,7 +15,10 @@ import { } from '../../../__tests__/feature/fapi'; import { openMenu } from '../../../__tests__/feature/menu'; import { renderWithClerk } from '../../../__tests__/feature/render'; +import { useModal } from '../../../modals/use-modal'; +import { useVirtualSegment } from '../../../routing/virtual-store'; import { UserProfile } from '../user-profile'; +import { userProfileModal } from '../user-profile.modal'; import { signedIn } from '../user-profile-connected-accounts-section/__tests__/user-profile-connected-accounts.fixtures'; function serve({ userAPIKeysEnabled = false } = {}) { @@ -31,6 +34,17 @@ function tabNames() { return screen.getAllByRole('tab').map(tab => tab.textContent); } +function Entry({ base }: { base: string }) { + return {JSON.stringify(useVirtualSegment(base))}; +} + +const entry = () => JSON.parse(screen.getByTestId('entry').textContent ?? 'null'); + +function OpenProfile({ page }: { page: string }) { + const profile = useModal(userProfileModal); + return ; +} + describe('UserProfile', () => { it('shows the fallback while Clerk loads, then the account page', async () => { serve(); @@ -75,14 +89,10 @@ describe('UserProfile', () => { it('passes the API keys options to the API keys page', async () => { serve({ userAPIKeysEnabled: true }); - await renderWithClerk( - , - ); + await renderWithClerk(); const user = userEvent.setup(); + await user.click(screen.getByRole('tab', { name: 'API Keys' })); await screen.findByText('API keys loaded'); const dialog = await openDialog(user, screen.getByRole('button', { name: 'Create API key' }), { name: 'Add new API key', @@ -116,18 +126,39 @@ describe('UserProfile', () => { expect(screen.getByText('Usage this month')).toBeVisible(); }); - it('opens on the default page without reordering the navigation', async () => { - serve(); - await renderWithClerk(); - expect(tabNames()).toEqual(['Account', 'Security']); - expect(screen.getByRole('tab', { name: 'Security' })).toHaveAttribute('aria-selected', 'true'); - expect(screen.getByRole('heading', { level: 3, name: 'Security' })).toBeVisible(); + it('keeps the root page at the base and writes the segments legacy used', async () => { + serve({ userAPIKeysEnabled: true }); + await renderWithClerk( + <> + + + , + ); + const user = userEvent.setup(); + expect(entry()).toEqual([]); + + await user.click(screen.getByRole('tab', { name: 'API Keys' })); + expect(entry()).toEqual(['api-keys']); + + await user.click(screen.getByRole('tab', { name: 'Account' })); + expect(entry()).toEqual([]); }); - it('opens on the first page when the default page is unavailable', async () => { - serve({ userAPIKeysEnabled: false }); - await renderWithClerk(); - expect(screen.getByRole('tab', { name: 'Account' })).toHaveAttribute('aria-selected', 'true'); + it('opens as a modal on the requested page, writing the segment legacy used', async () => { + serve({ userAPIKeysEnabled: true }); + await renderWithClerk( + <> + + + , + ); + const user = userEvent.setup(); + + await user.click(screen.getByRole('button', { name: 'Open profile' })); + const dialog = await screen.findByRole('dialog'); + + expect(await within(dialog).findByRole('tab', { name: 'API Keys' })).toHaveAttribute('aria-selected', 'true'); + expect(entry()).toEqual(['api-keys']); }); it('keeps its page when remounted under the same provider', async () => { @@ -142,22 +173,17 @@ describe('UserProfile', () => { expect(screen.getByRole('heading', { level: 3, name: 'Security' })).toBeVisible(); }); - it('falls back to the default page when the current page disappears', async () => { + it('falls back to the first page when the current page disappears', async () => { serve({ userAPIKeysEnabled: true }); - const { rerender } = await renderWithClerk(); + const { rerender } = await renderWithClerk(); const user = userEvent.setup(); await user.click(screen.getByRole('tab', { name: 'API Keys' })); expect(screen.getByRole('tab', { name: 'API Keys' })).toHaveAttribute('aria-selected', 'true'); - rerender( - , - ); + rerender(); - await waitFor(() => expect(screen.getByRole('tab', { name: 'Security' })).toHaveAttribute('aria-selected', 'true')); + await waitFor(() => expect(screen.getByRole('tab', { name: 'Account' })).toHaveAttribute('aria-selected', 'true')); expect(tabNames()).toEqual(['Account', 'Security']); }); @@ -246,12 +272,10 @@ describe('UserProfile with an active organization', () => { it('sends the user to afterLeaveOrganizationUrl after leaving the organization', async () => { const fapi = serveWithOrganization(); const { navigate } = await renderWithClerk( - , + , ); const user = userEvent.setup(); + await user.click(screen.getByRole('tab', { name: 'General', description: 'Organization' })); const dialog = await openDialog(user, await screen.findByRole('button', { name: 'Leave organization' }), { role: 'alertdialog', }); @@ -275,9 +299,10 @@ describe('UserProfile with an active organization', () => { expect(tabNames()).toEqual(['Account', 'Security']); }); - it('opens on an organization page by its prefixed id', async () => { + it('switches to an organization page from the navigation', async () => { serveWithOrganization(); - await renderWithClerk(); + await renderWithClerk(); + await userEvent.setup().click(screen.getByRole('tab', { name: 'Members', description: 'Organization' })); expect(screen.getByRole('tab', { name: 'Members', description: 'Organization' })).toHaveAttribute( 'aria-selected', 'true', @@ -304,12 +329,12 @@ describe('UserProfile with an active organization', () => { serveWithOrganization(); await renderWithClerk( Imposter content

}]} />, ); expect(tabNames()).toEqual(['Account', 'Security', 'General', 'Members']); + await userEvent.setup().click(screen.getByRole('tab', { name: 'General', description: 'Organization' })); expect(screen.getByRole('tab', { name: 'General', description: 'Organization' })).toHaveAttribute( 'aria-selected', 'true', @@ -342,7 +367,8 @@ describe('UserProfile with an active organization', () => { client: fapiClient([fapiSession({ id: 'sess_1', user, last_active_organization_id: acme.id })]), memberships, }); - const { clerk } = await renderWithClerk(); + const { clerk } = await renderWithClerk(); + await userEvent.setup().click(screen.getByRole('tab', { name: 'Members', description: 'Organization' })); await act(() => clerk.setActive({ organization: globex.id })); await waitFor(() => expect(tabsDescribedBy('Organization')).toEqual(['General'])); @@ -367,7 +393,8 @@ describe('UserProfile with an active organization', () => { client: fapiClient([fapiSession({ id: 'sess_1', user, last_active_organization_id: acme.id })]), memberships, }); - const { clerk } = await renderWithClerk(); + const { clerk } = await renderWithClerk(); + await userEvent.setup().click(screen.getByRole('tab', { name: 'Members', description: 'Organization' })); await act(() => clerk.setActive({ organization: globex.id })); await waitFor(() => expect(clerk.organization?.id).toBe(globex.id)); diff --git a/packages/mosaic/src/features/user-profile/user-profile.modal-content.tsx b/packages/mosaic/src/features/user-profile/user-profile.modal-content.tsx index 32c8f1d907a..fab98950e9d 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.modal-content.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile.modal-content.tsx @@ -18,13 +18,12 @@ export default function UserProfileModalContent({ } }, [signedOut, close]); - const { page, ...props } = options ?? {}; + const { page: _page, ...props } = options ?? {}; return ( ); } diff --git a/packages/mosaic/src/features/user-profile/user-profile.modal.ts b/packages/mosaic/src/features/user-profile/user-profile.modal.ts index 3f7fa7715d2..cef6dc0e3a3 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.modal.ts +++ b/packages/mosaic/src/features/user-profile/user-profile.modal.ts @@ -1,4 +1,5 @@ import { defineModalSurface } from '../../modals/define-modal-surface'; +import { pageParts } from '../profile-pages/profile-pages.layout'; import type { UserProfileModalOptions } from './user-profile.modal.types'; export const userProfileModal = defineModalSurface({ @@ -6,5 +7,5 @@ export const userProfileModal = defineModalSurface({ variant: 'profile', load: () => import('./user-profile.modal-content'), path: '/modal/user', - initialParts: options => options?.page?.split('/').filter(Boolean) ?? [], + initialParts: options => pageParts(options?.page), }); diff --git a/packages/mosaic/src/features/user-profile/user-profile.modal.types.ts b/packages/mosaic/src/features/user-profile/user-profile.modal.types.ts index c1c8d1e51b0..0ada402a43a 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.modal.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile.modal.types.ts @@ -1,11 +1,12 @@ import type { ModalContentProps } from '../../modals/modal.types'; import type { UserProfileProps } from './user-profile'; +import type { UserProfileViewProps } from './user-profile.view'; export interface UserProfileModalOptions extends Pick< UserProfileProps, 'customPages' | 'pageOrder' | 'additionalOAuthScopes' | 'apiKeysProps' | 'combined' | 'organizationProfileProps' > { - page?: UserProfileProps['defaultPage']; + page?: UserProfileViewProps['activePage']; } export type UserProfileModalContentProps = ModalContentProps; diff --git a/packages/mosaic/src/features/user-profile/user-profile.tsx b/packages/mosaic/src/features/user-profile/user-profile.tsx index 0b5869cc6e5..aa9e9145da8 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile.tsx @@ -23,14 +23,6 @@ import { useUserProfileUserModel } from './user-profile-user.model'; /** Everything `` takes: your own pages and their order, the page it opens on, and the OAuth scopes it asks for. */ export interface UserProfileProps extends Pick, Pick { - /** - * The page the profile opens on: a built-in page's id, or a custom page's `path`. Unlike - * `pageOrder`, it leaves the navigation order alone. A page the navigation does not list opens - * the first page instead. - * - * @default the first page in the navigation - */ - defaultPage?: UserProfileViewProps['activePage']; /** * The base the profile's pages are nested under. Two profiles on one page need different bases. * @@ -82,13 +74,6 @@ export interface UserProfileProps * ``` * * @example - * `defaultPage` picks the page the profile opens on without moving it in the navigation. An - * organization page's id is prefixed with `organization/`. - * ```tsx - * - * ``` - * - * @example * `combined={false}` leaves the active organization's pages out. * ```tsx * @@ -162,7 +147,6 @@ export function UserProfileSurface({ organizationProfileProps, customPages, pageOrder, - defaultPage, path = '/user', additionalOAuthScopes, fallback, @@ -181,7 +165,6 @@ export function UserProfileSurface({ const viewProps: AccountViewProps = { elevation: 'flush', path, - initialPage: defaultPage ?? pageOrder?.[0] ?? 'account', customPages, pageOrder, pages: { @@ -210,12 +193,11 @@ export function UserProfileSurface({ } type AccountViewProps = Omit & { - initialPage: UserProfileViewProps['activePage']; path: string; }; -function AccountUserProfileView({ initialPage, path, ...viewProps }: AccountViewProps) { - const { activePage, onPageChange } = useProfilePagesController(initialPage, userProfilePageIds(viewProps), { +function AccountUserProfileView({ path, ...viewProps }: AccountViewProps) { + const { activePage, onPageChange } = useProfilePagesController(userProfilePageIds(viewProps), { routing: 'virtual', path, }); @@ -230,7 +212,6 @@ function AccountUserProfileView({ initialPage, path, ...viewProps }: AccountView } function CombinedUserProfileView({ - initialPage, path, organizationProfileProps: { customPages, pageOrder, apiKeysProps, afterLeaveOrganizationUrl } = {}, ...viewProps @@ -240,11 +221,10 @@ function CombinedUserProfileView({ model.status === 'ready' ? { pages: organizationProfilePanels(model.pages, apiKeysProps), customPages, pageOrder } : undefined; - const { activePage, onPageChange } = useProfilePagesController( - initialPage, - model.status === 'loading' ? undefined : userProfilePageIds({ ...viewProps, organization }), - { routing: 'virtual', path }, - ); + const { activePage, onPageChange } = useProfilePagesController(userProfilePageIds({ ...viewProps, organization }), { + routing: 'virtual', + path, + }); return ( From 43ae635ad581b26b82c677ab4855f594557e40df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Thu, 5 Nov 2026 10:57:55 +0100 Subject: [PATCH 11/26] Add oxlint config for routing folder --- packages/mosaic/oxlint.config.mjs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/mosaic/oxlint.config.mjs b/packages/mosaic/oxlint.config.mjs index 4bd6515ab76..17f31843f0d 100644 --- a/packages/mosaic/oxlint.config.mjs +++ b/packages/mosaic/oxlint.config.mjs @@ -49,6 +49,7 @@ export default defineConfig({ })), { type: 'queries', pattern: 'src/queries', partialMatch: false }, { type: 'modals', pattern: 'src/modals', partialMatch: false }, + { type: 'routing', pattern: 'src/routing', partialMatch: false }, { type: 'blocks', pattern: 'src/blocks', partialMatch: false }, { type: 'features', pattern: 'src/features/*', partialMatch: false, capture: ['feature'] }, { type: 'root', pattern: 'src', partialMatch: false }, @@ -286,7 +287,7 @@ export default defineConfig({ checkUnknownLocals: true, checkInternals: true, message: - 'Mosaic layers import down: features, then blocks, modals and queries, then components, hooks, utils and styles, then primitives. Move the shared piece down a layer, or pass it in as a prop.', + 'Mosaic layers import down: features, then blocks, modals, queries and routing, then components, hooks, utils and styles, then primitives. Move the shared piece down a layer, or pass it in as a prop.', policies: [ { allow: { to: { module: { origin: ['external', 'core'] } } } }, { allow: { to: { element: { type: '{{ from.element.type }}' } } } }, @@ -296,6 +297,7 @@ export default defineConfig({ from: { element: { type: ['utils', 'hooks', 'styles', 'components', 'localization', 'modals'] } }, allow: { to: { element: { type: SHARED } } }, }, + { from: { element: { type: 'modals' } }, allow: { to: { element: { type: 'routing' } } } }, { from: { element: { type: 'localization' } }, allow: { to: { file: { categories: 'messages' } } }, @@ -309,7 +311,7 @@ export default defineConfig({ }, { from: { element: { type: 'features' } }, - allow: { to: { element: { type: [...SHARED, 'queries', 'modals', 'blocks'] } } }, + allow: { to: { element: { type: [...SHARED, 'queries', 'modals', 'blocks', 'routing'] } } }, }, { from: FLOW_UI, From 73e3fe0a253054b43f79e3fc880cfefd67bde0b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Thu, 5 Nov 2026 14:12:40 +0100 Subject: [PATCH 12/26] Fix self-review feedback --- .changeset/curly-books-pull.md | 23 ++++++++ .../profile-pages.controller.test.tsx | 38 +++++++++++++ .../profile-pages/profile-pages.layout.ts | 11 +++- .../__tests__/user-profile.layout.test.ts | 22 ++++++-- .../src/routing/__tests__/routes.test.ts | 13 ++--- .../src/routing/__tests__/use-router.test.tsx | 14 +++-- packages/mosaic/src/routing/routes.ts | 20 ++++--- packages/mosaic/src/routing/use-router.ts | 54 ++++++++++++------- packages/swingset/tsconfig.json | 23 +++----- 9 files changed, 161 insertions(+), 57 deletions(-) create mode 100644 .changeset/curly-books-pull.md create mode 100644 packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx diff --git a/.changeset/curly-books-pull.md b/.changeset/curly-books-pull.md new file mode 100644 index 00000000000..fa724593c4c --- /dev/null +++ b/.changeset/curly-books-pull.md @@ -0,0 +1,23 @@ +--- +'@clerk/mosaic': minor +--- + +`UserProfile` and `OrganizationProfile` now route their pages through a location instead of local state, which changes a few things for consumers: + +- The `defaultPage` prop is removed. A profile opens on the first page in its navigation, as the legacy components do. To open on another page, put it first with `pageOrder`, or open the profile as a modal with the `page` option: + + ```tsx + // before + ; + + // after + ; + // or + const userProfile = useModal(userProfileModal); + userProfile.open({ page: 'security' }); + ``` + +- A new `path` prop sets the base a profile's pages live under, `/user` and `/organization` by default. Two profiles on one page need different bases. +- The open page outlives the component: a profile that unmounts and mounts again under the same `MosaicProvider` comes back on the page it was on. +- A profile opened as a modal starts on the requested page every time, and closing it no longer leaves the page behind for the next open. +- A custom page whose `path` cannot be a route, because it is percent-encoded, starts with a colon, or is the URL segment of a built-in page such as `api-keys`, is left out of the navigation with a console warning instead of rendering a page that cannot be reached. diff --git a/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx b/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx new file mode 100644 index 00000000000..c7c26409076 --- /dev/null +++ b/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx @@ -0,0 +1,38 @@ +import { logger } from '@clerk/shared/logger'; +import { renderHook } from '@testing-library/react'; +import type { ReactNode } from 'react'; +import { describe, expect, it, vi } from 'vitest'; + +import { useVirtualSegment, VirtualStoreProvider } from '../../../routing/virtual-store'; +import { useProfilePagesController } from '../profile-pages.controller'; + +const wrapper = ({ children }: { children: ReactNode }) => {children}; + +describe('useProfilePagesController', () => { + it('routes nothing and warns about nothing while there are no pages', () => { + const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); + const { result } = renderHook( + () => ({ + pages: useProfilePagesController([], { routing: 'virtual', path: '/organization' }), + segment: useVirtualSegment('/organization'), + }), + { wrapper }, + ); + + expect(result.current.segment).toEqual([]); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + it('makes the first page the root and gives the others their segments', () => { + const { result } = renderHook( + () => ({ + pages: useProfilePagesController(['general', 'apiKeys'], { routing: 'virtual', path: '/organization' }), + segment: useVirtualSegment('/organization'), + }), + { wrapper }, + ); + expect(result.current.pages.activePage).toBe('general'); + expect(result.current.segment).toEqual([]); + }); +}); diff --git a/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts b/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts index 1338c947eeb..c384566d88c 100644 --- a/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts +++ b/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts @@ -15,13 +15,22 @@ export function pageParts(page: string | undefined): string[] { return page === undefined ? [] : pageSegment(page).split('/').filter(Boolean); } +const BUILT_IN_SEGMENT_IDS = new Map(Object.entries(BUILT_IN_SEGMENTS).map(([id, segment]) => [segment, id])); + export function withRoutablePaths(pages: readonly Page[]): Page[] { return pages.filter(page => { + const builtIn = BUILT_IN_SEGMENT_IDS.get(page.path); + if (builtIn !== undefined) { + logger.warnOnce( + `[Clerk] Custom page path "${page.path}" is the segment of the built-in "${builtIn}" page, so the page is left out. Use "${builtIn}" as the path to replace that page.`, + ); + return false; + } if (isLiteralPattern(page.path)) { return true; } logger.warnOnce( - `[Clerk] Custom page path "${page.path}" is not a plain URL segment, so the page is left out. Use URL-safe characters only, without a leading colon.`, + `[Clerk] Custom page path "${page.path}" cannot be a route, so the page is left out. Write it decoded, without a leading colon.`, ); return false; }); diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts b/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts index 462b251e5c9..08fd1ba2a3f 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts @@ -39,15 +39,31 @@ describe('resolveUserProfilePages', () => { it('leaves out a custom page whose path cannot be a route, and warns', () => { const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); - const spaced: CustomProfilePage = { label: 'My page', path: 'my page', content: null }; + const encoded: CustomProfilePage = { label: 'My page', path: 'my%20page', content: null }; const param: CustomProfilePage = { label: 'Param', path: ':id', content: null }; + const spaced: CustomProfilePage = { label: 'Spaced', path: 'my page', content: null }; - expect(resolveUserProfilePages(['account'], [spaced, terms, param])).toEqual([ + expect(resolveUserProfilePages(['account'], [encoded, terms, param, spaced])).toEqual([ { id: 'account' }, { id: 'terms', custom: terms }, + { id: 'my page', custom: spaced }, ]); expect(warn).toHaveBeenCalledTimes(2); - expect(warn.mock.calls[0][0]).toContain('"my page"'); + expect(warn.mock.calls[0][0]).toContain('"my%20page"'); + warn.mockRestore(); + }); + + it("leaves out a custom page whose path is a built-in page's segment, and warns", () => { + const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); + const apiKeys: CustomProfilePage = { label: 'Keys', path: 'api-keys', content: null }; + + expect(resolveUserProfilePages(['account', 'apiKeys'], [apiKeys, terms])).toEqual([ + { id: 'account' }, + { id: 'apiKeys' }, + { id: 'terms', custom: terms }, + ]); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toContain('"apiKeys"'); warn.mockRestore(); }); diff --git a/packages/mosaic/src/routing/__tests__/routes.test.ts b/packages/mosaic/src/routing/__tests__/routes.test.ts index 53da99d423f..dd50f7c0ee4 100644 --- a/packages/mosaic/src/routing/__tests__/routes.test.ts +++ b/packages/mosaic/src/routing/__tests__/routes.test.ts @@ -66,24 +66,25 @@ describe('compilePattern', () => { ]); }); - it('rejects a literal that encoding would change', () => { - expect(() => compilePattern('api keys')).toThrow('must be plain'); - expect(() => compilePattern('api%20keys')).toThrow('must be plain'); + it('rejects an encoded literal', () => { + expect(compilePattern('api keys')).toEqual([{ param: false, value: 'api keys' }]); + expect(() => compilePattern('api%20keys')).toThrow('written decoded'); }); }); describe('isLiteralPattern', () => { - it('accepts plain parts', () => { + it('accepts decoded parts', () => { expect(isLiteralPattern('api-keys')).toBe(true); expect(isLiteralPattern('organization/general')).toBe(true); + expect(isLiteralPattern('my page')).toBe(true); + expect(isLiteralPattern('café')).toBe(true); }); - it('rejects an empty pattern, a param, and a literal that encoding would change', () => { + it('rejects an empty pattern, a param, and an encoded literal', () => { expect(isLiteralPattern('')).toBe(false); expect(isLiteralPattern('/')).toBe(false); expect(isLiteralPattern(':id')).toBe(false); expect(isLiteralPattern('terms/:id?')).toBe(false); - expect(isLiteralPattern('my page')).toBe(false); expect(isLiteralPattern('my%20page')).toBe(false); }); }); diff --git a/packages/mosaic/src/routing/__tests__/use-router.test.tsx b/packages/mosaic/src/routing/__tests__/use-router.test.tsx index 31fbca7b86d..a43c80e9e15 100644 --- a/packages/mosaic/src/routing/__tests__/use-router.test.tsx +++ b/packages/mosaic/src/routing/__tests__/use-router.test.tsx @@ -148,7 +148,7 @@ describe('useRouterScope', () => { warn.mockRestore(); }); - it('redirects under StrictMode without mistaking its own repeated effect for a collision', () => { + it('redirects under StrictMode without warning', () => { configure({ reactStrictMode: true }); onTestFinished(() => configure({ reactStrictMode: false })); const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); @@ -179,7 +179,7 @@ describe('useRouterScope', () => { warn.mockRestore(); }); - it('stops redirecting when another owner of the same base overwrites its redirect, and warns', () => { + it('stops redirecting after a burst when two owners of one base redirect each other, and warns', () => { const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); const orgRoutes = { general: 'general', members: 'members' } as const; const { result } = renderHook( @@ -191,11 +191,17 @@ describe('useRouterScope', () => { { wrapper: wrapperWith() }, ); - expect(result.current.segment).toEqual(['general']); + expect(['account', 'general']).toContain(result.current.segment[0]); expect(result.current.user.route).toBe('account'); expect(result.current.org.route).toBe('general'); expect(warn).toHaveBeenCalledTimes(1); - expect(warn.mock.calls[0][0]).toContain('wrote "general" instead'); + expect(warn.mock.calls[0][0]).toContain('Stopped redirecting under "/user"'); + + act(() => { + result.current.user.go('security'); + }); + + expect(['security', 'general']).toContain(result.current.segment[0]); warn.mockRestore(); }); diff --git a/packages/mosaic/src/routing/routes.ts b/packages/mosaic/src/routing/routes.ts index 5f10ecbacd3..1a7075dafb0 100644 --- a/packages/mosaic/src/routing/routes.ts +++ b/packages/mosaic/src/routing/routes.ts @@ -50,25 +50,31 @@ function partsOf(pattern: string): string[] { return pattern.split('/').filter(Boolean); } -// Literals are compared against decoded parts, so 'api keys' or 'api%20keys' could never match the way they read. -function assertPlainLiteral(part: string): void { - if (encodeURIComponent(part) !== part) { - throw new Error(`[clerk] Route pattern part "${part}" must be plain; patterns are matched against decoded parts.`); +// Literals are compared against decoded parts, so an encoded literal such as 'api%20keys' could never match the way it reads. +function isDecodedLiteral(part: string): boolean { + return !part.includes('%'); +} + +function assertDecodedLiteral(part: string): void { + if (!isDecodedLiteral(part)) { + throw new Error( + `[Clerk] Route pattern part "${part}" must be written decoded; patterns are matched against decoded parts.`, + ); } } -// 'api-keys' -> true 'my page' -> false ':id' -> false '' -> false +// 'api-keys' -> true 'my page' -> true 'my%20page' -> false ':id' -> false '' -> false // Whether a string from outside, such as a custom page path, can be used as a literal route as is. export function isLiteralPattern(pattern: string): boolean { const parts = partsOf(pattern); - return parts.length > 0 && parts.every(part => !part.startsWith(':') && encodeURIComponent(part) === part); + return parts.length > 0 && parts.every(part => !part.startsWith(':') && isDecodedLiteral(part)); } // 'billing/plans/:planId?' -> [{ param: false, value: 'billing' }, { param: false, value: 'plans' }, { param: true, name: 'planId', optional: true }] export function compilePattern(pattern: string): PatternPart[] { return partsOf(pattern).map(part => { if (!part.startsWith(':')) { - assertPlainLiteral(part); + assertDecodedLiteral(part); return { param: false, value: part }; } const optional = part.endsWith('?'); diff --git a/packages/mosaic/src/routing/use-router.ts b/packages/mosaic/src/routing/use-router.ts index 0d8a0157198..3d10214189f 100644 --- a/packages/mosaic/src/routing/use-router.ts +++ b/packages/mosaic/src/routing/use-router.ts @@ -1,5 +1,5 @@ import { logger } from '@clerk/shared/logger'; -import { useEffect, useMemo, useRef, useState } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import type { MosaicLocation, SegmentParts } from './location'; import type { Route, Router } from './router'; @@ -31,8 +31,32 @@ function useStableTable(routes: Routes): Routes { return useStableByKey(routes, Object.entries(routes).flat().join(KEY_SEPARATOR)); } -function sameParts(a: SegmentParts, b: SegmentParts): boolean { - return a.length === b.length && a.every((part, index) => part === b[index]); +const REDIRECT_LIMIT = 10; +const REDIRECT_WINDOW_MS = 1000; + +// A write that gives up after a burst. Not-found redirects are automatic, so two scopes with +// different fallbacks on one path would redirect each other forever; a scope that has redirected +// this many times within the window stops and warns instead. No real navigation comes close. +// All attempts to fix this in a more exact fashion turned out to be unnecessarily complex, we only +// need a heuristic to not crash and warn on misconfigurations, not an exact fix. +function withRedirectLimit(location: MosaicLocation, path: string | undefined): MosaicLocation['write'] { + let count = 0; + let since = 0; + return (parts, options) => { + const now = Date.now(); + if (now - since > REDIRECT_WINDOW_MS) { + count = 0; + since = now; + } + count += 1; + if (count > REDIRECT_LIMIT) { + logger.warnOnce( + `[Clerk] Stopped redirecting under "${path ?? '/'}" after ${REDIRECT_LIMIT} redirects within a second. Two components are probably configured with this path; give one of them a different path.`, + ); + return; + } + location.write(parts, options); + }; } function missingScope(): never { @@ -89,30 +113,22 @@ export function useRouterScope( return router.parse(target) ? target : undefined; }, [table, router, notFound]); + const redirect = useMemo(() => withRedirectLimit(location, path), [location, path]); const redirectTo = matched === undefined ? notFound : undefined; - const redirect = useRef<{ from: SegmentParts; to: SegmentParts }>(undefined); useEffect(() => { - const last = redirect.current; - redirect.current = undefined; if (redirectTo === undefined) { return; } if (fallback === undefined) { - logger.warnOnce( - `[Clerk] The notFound route "${redirectTo}" under "${path ?? '/'}" is either missing or needs params, so it cannot be a fallback. Use a route without params.`, - ); - return; - } - if (last && !sameParts(last.from, parts) && !sameParts(last.to, parts)) { - // This breaks what could otherwise be an infinite redirect loop - logger.warnOnce( - `[Clerk] The segment "${last.from.join('/')}" under "${path ?? '/'}" was not found, but after updating the location to match the fallback page, something wrote "${parts.join('/')}" instead. Two routers probably share this base; give one of them a different path.`, - ); + if (Object.keys(table).length > 0) { + logger.warnOnce( + `[Clerk] The notFound route "${redirectTo}" under "${path ?? '/'}" is either missing or needs params, so it cannot be a fallback. Use a route without params.`, + ); + } return; } - redirect.current = { from: parts, to: fallback }; - location.write(fallback, { replace: true }); - }, [parts, redirectTo, fallback, location, path]); + redirect(fallback, { replace: true }); + }, [parts, redirectTo, fallback, redirect, table, path]); return useMemo( () => ({ route: current.name, params: current.params, go: router.go, scope }), diff --git a/packages/swingset/tsconfig.json b/packages/swingset/tsconfig.json index 643897f3f4c..ebb3a20eb3a 100644 --- a/packages/swingset/tsconfig.json +++ b/packages/swingset/tsconfig.json @@ -3,25 +3,16 @@ "target": "ES2022", "module": "preserve", "moduleResolution": "bundler", - "jsx": "preserve", + "jsx": "react-jsx", "strict": true, "skipLibCheck": true, "verbatimModuleSyntax": true, "moduleDetection": "force", - "lib": [ - "ES2023", - "DOM" - ], + "lib": ["ES2023", "DOM"], "paths": { - "@/*": [ - "./src/*" - ], - "@clerk/mosaic": [ - "../mosaic/src" - ], - "@clerk/mosaic/*": [ - "../mosaic/src/*" - ] + "@/*": ["./src/*"], + "@clerk/mosaic": ["../mosaic/src"], + "@clerk/mosaic/*": ["../mosaic/src/*"] }, "plugins": [ { @@ -41,7 +32,5 @@ ".next/types/**/*.ts", ".next/dev/types/**/*.ts" ], - "exclude": [ - "node_modules" - ] + "exclude": ["node_modules"] } From 2114286a3c25e3623595d1f13d86af125ab73539 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Thu, 5 Nov 2026 15:09:02 +0100 Subject: [PATCH 13/26] More self-review tweaks --- .changeset/curly-books-pull.md | 15 +---- .../organization-profile.feature.test.tsx | 60 +++++++++++++++++++ .../profile-pages/profile-pages.controller.ts | 4 +- .../__tests__/user-profile.feature.test.tsx | 51 ++++++++++++++-- 4 files changed, 110 insertions(+), 20 deletions(-) diff --git a/.changeset/curly-books-pull.md b/.changeset/curly-books-pull.md index fa724593c4c..149e7287fff 100644 --- a/.changeset/curly-books-pull.md +++ b/.changeset/curly-books-pull.md @@ -4,20 +4,9 @@ `UserProfile` and `OrganizationProfile` now route their pages through a location instead of local state, which changes a few things for consumers: -- The `defaultPage` prop is removed. A profile opens on the first page in its navigation, as the legacy components do. To open on another page, put it first with `pageOrder`, or open the profile as a modal with the `page` option: - - ```tsx - // before - ; - - // after - ; - // or - const userProfile = useModal(userProfileModal); - userProfile.open({ page: 'security' }); - ``` - +- The `defaultPage` prop is removed. A profile opens on the first page in its navigation, as the legacy components do. To open on another page, put it first with `pageOrder`, or open the profile as a modal with the `page` option. - A new `path` prop sets the base a profile's pages live under, `/user` and `/organization` by default. Two profiles on one page need different bases. +- There is no path or hash mode yet, so urls are not updated, this will come in a later version. - The open page outlives the component: a profile that unmounts and mounts again under the same `MosaicProvider` comes back on the page it was on. - A profile opened as a modal starts on the requested page every time, and closing it no longer leaves the page behind for the next open. - A custom page whose `path` cannot be a route, because it is percent-encoded, starts with a colon, or is the URL segment of a built-in page such as `api-keys`, is left out of the navigation with a console warning instead of rendering a page that cannot be reached. diff --git a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.feature.test.tsx b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.feature.test.tsx index a5075be1efc..66829ca1316 100644 --- a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.feature.test.tsx +++ b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.feature.test.tsx @@ -14,7 +14,10 @@ import { fapiUser, } from '../../../__tests__/feature/fapi'; import { renderWithClerk } from '../../../__tests__/feature/render'; +import { useModal } from '../../../modals/use-modal'; +import { useVirtualEntries } from '../../../routing/virtual-store'; import { OrganizationProfile } from '../organization-profile'; +import { organizationProfileModal } from '../organization-profile.modal'; const acme = fapiOrganization({ id: 'org_1', name: 'Acme', slug: 'acme' }); @@ -46,6 +49,17 @@ function tabNames() { .map(tab => tab.textContent); } +function Entry({ base }: { base: string }) { + return {JSON.stringify(useVirtualEntries().get(base) ?? null)}; +} + +const entry = () => JSON.parse(screen.getByTestId('entry').textContent ?? 'null'); + +function OpenProfile({ page }: { page: string }) { + const profile = useModal(organizationProfileModal); + return ; +} + describe('OrganizationProfile', () => { it('shows the fallback while Clerk loads, then the general page', async () => { serve(); @@ -119,6 +133,52 @@ describe('OrganizationProfile', () => { expect(within(dialog).getByRole('textbox', { name: 'Description' })).toBeVisible(); }); + it('keeps its page when remounted under the same provider', async () => { + serve(); + const { rerender } = await renderWithClerk(); + const user = userEvent.setup(); + + await user.click(profileNav().getByRole('tab', { name: 'Members' })); + rerender(); + + expect(profileNav().getByRole('tab', { name: 'Members' })).toHaveAttribute('aria-selected', 'true'); + expect(screen.getByRole('heading', { level: 3, name: 'Members' })).toBeVisible(); + }); + + it('routes two profiles on one page independently when they have different bases', async () => { + serve(); + await renderWithClerk( + <> + + + , + ); + const user = userEvent.setup(); + const [first, second] = screen.getAllByRole('navigation', { name: 'Organization' }).map(nav => within(nav)); + + await user.click(first.getByRole('tab', { name: 'Members' })); + + expect(first.getByRole('tab', { name: 'Members' })).toHaveAttribute('aria-selected', 'true'); + expect(second.getByRole('tab', { name: 'General' })).toHaveAttribute('aria-selected', 'true'); + }); + + it('opens as a modal on the requested page, writing the segment legacy used', async () => { + serve({ orgAPIKeysEnabled: true, permissions: ['org:sys_memberships:read', 'org:sys_api_keys:read'] }); + await renderWithClerk( + <> + + + , + ); + const user = userEvent.setup(); + + await user.click(screen.getByRole('button', { name: 'Open profile' })); + const dialog = await screen.findByRole('dialog'); + + expect(await within(dialog).findByRole('tab', { name: 'API Keys' })).toHaveAttribute('aria-selected', 'true'); + expect(entry()).toEqual(['api-keys']); + }); + it('switches pages from the navigation', async () => { serve({ orgAPIKeysEnabled: true, permissions: ['org:sys_memberships:read', 'org:sys_api_keys:read'] }); await renderWithClerk(); diff --git a/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts b/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts index 2382e8f5740..1797480694c 100644 --- a/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts +++ b/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts @@ -4,11 +4,11 @@ import { pageSegment } from './profile-pages.layout'; const NO_PAGE = ''; -export function useProfilePagesController(pageIds: readonly string[], routing: MosaicLocationOptions) { +export function useProfilePagesController(pageIds: readonly string[], options: MosaicLocationOptions) { const rootPage = pageIds[0] ?? NO_PAGE; const { route, go } = useRouterScope( Object.fromEntries(pageIds.map(id => [id, id === rootPage ? '' : pageSegment(id)])), - { ...routing, notFound: rootPage }, + { ...options, notFound: rootPage }, ); return { activePage: route, onPageChange: go }; diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx index bdf0c948072..4237f6d1a42 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx @@ -16,7 +16,7 @@ import { import { openMenu } from '../../../__tests__/feature/menu'; import { renderWithClerk } from '../../../__tests__/feature/render'; import { useModal } from '../../../modals/use-modal'; -import { useVirtualSegment } from '../../../routing/virtual-store'; +import { useVirtualEntries } from '../../../routing/virtual-store'; import { UserProfile } from '../user-profile'; import { userProfileModal } from '../user-profile.modal'; import { signedIn } from '../user-profile-connected-accounts-section/__tests__/user-profile-connected-accounts.fixtures'; @@ -35,14 +35,19 @@ function tabNames() { } function Entry({ base }: { base: string }) { - return {JSON.stringify(useVirtualSegment(base))}; + return {JSON.stringify(useVirtualEntries().get(base) ?? null)}; } const entry = () => JSON.parse(screen.getByTestId('entry').textContent ?? 'null'); -function OpenProfile({ page }: { page: string }) { +function OpenProfile({ page }: { page?: string }) { const profile = useModal(userProfileModal); - return ; + return ( + <> + + + + ); } describe('UserProfile', () => { @@ -135,7 +140,7 @@ describe('UserProfile', () => { , ); const user = userEvent.setup(); - expect(entry()).toEqual([]); + expect(entry()).toBeNull(); await user.click(screen.getByRole('tab', { name: 'API Keys' })); expect(entry()).toEqual(['api-keys']); @@ -161,6 +166,42 @@ describe('UserProfile', () => { expect(entry()).toEqual(['api-keys']); }); + it('opens as a modal on the root page with an empty entry', async () => { + serve(); + await renderWithClerk( + <> + + + , + ); + const user = userEvent.setup(); + + await user.click(screen.getByRole('button', { name: 'Open profile' })); + const dialog = await screen.findByRole('dialog'); + + expect(await within(dialog).findByRole('tab', { name: 'Account' })).toHaveAttribute('aria-selected', 'true'); + await waitFor(() => expect(entry()).toEqual([])); + }); + + it('clears the modal entry when a profile closes, so it reopens on the first page', async () => { + serve(); + await renderWithClerk( + <> + + + , + ); + const user = userEvent.setup(); + + await user.click(screen.getByRole('button', { name: 'Open profile' })); + const dialog = await screen.findByRole('dialog'); + expect(await within(dialog).findByRole('tab', { name: 'Security' })).toHaveAttribute('aria-selected', 'true'); + + await user.click(screen.getByRole('button', { name: 'Close profile' })); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + expect(entry()).toBeNull(); + }); + it('keeps its page when remounted under the same provider', async () => { serve(); const { rerender } = await renderWithClerk(); From 7aeab797322d8eb5eab7f7d40800c58c1da5fdf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Fri, 6 Nov 2026 13:52:20 +0100 Subject: [PATCH 14/26] Make virtual routing behave like a URL --- .changeset/curly-books-pull.md | 2 - .../organization-profile.feature.test.tsx | 7 +- .../profile-pages.controller.test.tsx | 4 +- .../__tests__/user-profile.feature.test.tsx | 7 +- packages/mosaic/src/modals/modal-host.tsx | 38 ++- packages/mosaic/src/modals/modal.types.ts | 6 +- packages/mosaic/src/modals/use-modal.test.tsx | 8 +- .../__tests__/location-contract.test.tsx | 315 ++++++++++++++++++ .../src/routing/__tests__/router.test.ts | 1 + .../__tests__/virtual-location.test.tsx | 89 ++++- packages/mosaic/src/routing/location.ts | 13 +- packages/mosaic/src/routing/routes.test-d.ts | 2 +- .../mosaic/src/routing/use-mosaic-location.ts | 6 +- packages/mosaic/src/routing/use-router.ts | 48 ++- .../mosaic/src/routing/virtual-location.ts | 9 +- packages/mosaic/src/routing/virtual-store.tsx | 83 +++-- 16 files changed, 559 insertions(+), 79 deletions(-) create mode 100644 packages/mosaic/src/routing/__tests__/location-contract.test.tsx diff --git a/.changeset/curly-books-pull.md b/.changeset/curly-books-pull.md index 149e7287fff..0677115fc02 100644 --- a/.changeset/curly-books-pull.md +++ b/.changeset/curly-books-pull.md @@ -7,6 +7,4 @@ - The `defaultPage` prop is removed. A profile opens on the first page in its navigation, as the legacy components do. To open on another page, put it first with `pageOrder`, or open the profile as a modal with the `page` option. - A new `path` prop sets the base a profile's pages live under, `/user` and `/organization` by default. Two profiles on one page need different bases. - There is no path or hash mode yet, so urls are not updated, this will come in a later version. -- The open page outlives the component: a profile that unmounts and mounts again under the same `MosaicProvider` comes back on the page it was on. -- A profile opened as a modal starts on the requested page every time, and closing it no longer leaves the page behind for the next open. - A custom page whose `path` cannot be a route, because it is percent-encoded, starts with a colon, or is the URL segment of a built-in page such as `api-keys`, is left out of the navigation with a console warning instead of rendering a page that cannot be reached. diff --git a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.feature.test.tsx b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.feature.test.tsx index 66829ca1316..25f801defdf 100644 --- a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.feature.test.tsx +++ b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.feature.test.tsx @@ -133,16 +133,17 @@ describe('OrganizationProfile', () => { expect(within(dialog).getByRole('textbox', { name: 'Description' })).toBeVisible(); }); - it('keeps its page when remounted under the same provider', async () => { + it('starts on the first page again after it was unmounted', async () => { serve(); const { rerender } = await renderWithClerk(); const user = userEvent.setup(); await user.click(profileNav().getByRole('tab', { name: 'Members' })); + rerender(

Elsewhere

); rerender(); - expect(profileNav().getByRole('tab', { name: 'Members' })).toHaveAttribute('aria-selected', 'true'); - expect(screen.getByRole('heading', { level: 3, name: 'Members' })).toBeVisible(); + expect(profileNav().getByRole('tab', { name: 'General' })).toHaveAttribute('aria-selected', 'true'); + expect(screen.getByRole('heading', { level: 3, name: 'General' })).toBeVisible(); }); it('routes two profiles on one page independently when they have different bases', async () => { diff --git a/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx b/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx index c7c26409076..4fff03d939c 100644 --- a/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx +++ b/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx @@ -19,7 +19,7 @@ describe('useProfilePagesController', () => { { wrapper }, ); - expect(result.current.segment).toEqual([]); + expect(result.current.segment).toBeUndefined(); expect(warn).not.toHaveBeenCalled(); warn.mockRestore(); }); @@ -33,6 +33,6 @@ describe('useProfilePagesController', () => { { wrapper }, ); expect(result.current.pages.activePage).toBe('general'); - expect(result.current.segment).toEqual([]); + expect(result.current.segment).toBeUndefined(); }); }); diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx index 4237f6d1a42..b9427f3f91d 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx @@ -202,16 +202,17 @@ describe('UserProfile', () => { expect(entry()).toBeNull(); }); - it('keeps its page when remounted under the same provider', async () => { + it('starts on the first page again after it was unmounted', async () => { serve(); const { rerender } = await renderWithClerk(); const user = userEvent.setup(); await user.click(screen.getByRole('tab', { name: 'Security' })); + rerender(

Elsewhere

); rerender(); - expect(screen.getByRole('tab', { name: 'Security' })).toHaveAttribute('aria-selected', 'true'); - expect(screen.getByRole('heading', { level: 3, name: 'Security' })).toBeVisible(); + expect(screen.getByRole('tab', { name: 'Account' })).toHaveAttribute('aria-selected', 'true'); + expect(screen.getByRole('heading', { level: 3, name: 'Account' })).toBeVisible(); }); it('falls back to the first page when the current page disappears', async () => { diff --git a/packages/mosaic/src/modals/modal-host.tsx b/packages/mosaic/src/modals/modal-host.tsx index fdb724ac938..9838ce724f6 100644 --- a/packages/mosaic/src/modals/modal-host.tsx +++ b/packages/mosaic/src/modals/modal-host.tsx @@ -37,34 +37,31 @@ export function ModalSurfaceHost({ children: React.ReactNode; }): React.ReactElement { const [state, setState] = React.useState>({ - open: false, options: undefined, container: undefined, openCount: 0, }); - const routed = surface.path !== undefined; - const location = useMosaicLocation({ routing: 'virtual', path: surface.path ?? `/modal/${surface.id}` }); + // The modal open state is derived from the routing state, so there is no state to sync and + // opening/closing is just writing to the appropriate routing state. This means that in a mode + // that writes to the url, the modal opens automatically on page load if the url says so, + // the Back button just works etc. The public modal open/close are just routing writes. + const base = surface.path ?? `/modal/${surface.id}`; + const location = useMosaicLocation({ routing: 'virtual', path: base }); + const open = location.useSegmentParts() !== undefined; const modal = React.useMemo>(() => { const preload = () => surface.loader.send({ type: 'LOAD' }); return { open: (options, container) => { preload(); - if (routed) { - location.write(surface.initialParts?.(options) ?? []); - } - setState(previous => ({ open: true, options, container, openCount: previous.openCount + 1 })); - }, - close: () => { - if (routed && state.open) { - location.clear(); - } - setState(previous => (previous.open ? { ...previous, open: false } : previous)); + setState(previous => ({ options, container, openCount: previous.openCount + 1 })); + location.write(surface.initialParts?.(options) ?? []); }, + close: () => location.clear(), preload, - isOpen: state.open, + isOpen: open, }; - }, [surface, routed, location, state.open]); + }, [surface, location, open]); return ( @@ -72,6 +69,8 @@ export function ModalSurfaceHost({ @@ -81,10 +80,14 @@ export function ModalSurfaceHost({ function ModalSlot({ surface, state, + open, + base, close, }: { surface: ModalSurface; state: ModalState; + open: boolean; + base: string; close: () => void; }) { const [content] = useActor(surface.loader); @@ -93,7 +96,7 @@ function ModalSlot({ const dialog = ( { if (!open) { close(); @@ -106,7 +109,7 @@ function ModalSlot({ key={state.openCount} options={state.options} close={close} - path={surface.path} + path={base} /> ) : failed ? ( @@ -130,7 +133,6 @@ function LoadFailed() { } interface ModalState { - open: boolean; options: Options | undefined; container: PortalContainer | undefined; openCount: number; diff --git a/packages/mosaic/src/modals/modal.types.ts b/packages/mosaic/src/modals/modal.types.ts index 6a30d891bfe..37669f88d2a 100644 --- a/packages/mosaic/src/modals/modal.types.ts +++ b/packages/mosaic/src/modals/modal.types.ts @@ -8,16 +8,16 @@ import type { ModalContentLoader } from './modal-content-loader'; export interface ModalContentProps { options: Options | undefined; close: () => void; - path: string | undefined; + path: string; } export interface ModalSurfaceConfig { id: string; variant: DialogVariant; load: () => Promise<{ default: ComponentType> }>; - // If path is provided, the modal host writes it before opening and clears it on close + // The base of the entry that keeps the modal open, `/modal/` by default path?: string; - // Optional way to derive the initial segment to be written together with path on open + // The parts written on open, derived from the open options; an empty entry by default. initialParts?: (options: Options | undefined) => SegmentParts; } diff --git a/packages/mosaic/src/modals/use-modal.test.tsx b/packages/mosaic/src/modals/use-modal.test.tsx index f3ee968e516..93ea682f130 100644 --- a/packages/mosaic/src/modals/use-modal.test.tsx +++ b/packages/mosaic/src/modals/use-modal.test.tsx @@ -464,7 +464,7 @@ describe('useModal', () => { describe('a modal surface that routes', () => { function Entry() { - return {JSON.stringify(useVirtualSegment('/modal/stub'))}; + return {JSON.stringify(useVirtualSegment('/modal/stub') ?? null)}; } function routedSurface() { @@ -521,7 +521,7 @@ describe('a modal surface that routes', () => { act(() => ref.handle?.close()); await flush(); - expect(entry()).toEqual([]); + expect(entry()).toBeNull(); act(() => ref.handle?.open()); await flush(); @@ -576,7 +576,7 @@ describe('a modal surface that routes', () => { act(() => ref.handle?.close()); await flush(); - expect(entry()).toEqual([]); + expect(entry()).toBeNull(); expect(warn).not.toHaveBeenCalled(); }); @@ -591,7 +591,7 @@ describe('a modal surface that routes', () => { await flush(); expect(screen.getByTestId('route')).toBeInTheDocument(); - expect(entry()).toEqual([]); + expect(entry()).toBeNull(); expect(warn).not.toHaveBeenCalled(); }); diff --git a/packages/mosaic/src/routing/__tests__/location-contract.test.tsx b/packages/mosaic/src/routing/__tests__/location-contract.test.tsx new file mode 100644 index 00000000000..b49a1066b9a --- /dev/null +++ b/packages/mosaic/src/routing/__tests__/location-contract.test.tsx @@ -0,0 +1,315 @@ +import { logger } from '@clerk/shared/logger'; +import { act, cleanup, configure, fireEvent, render, screen } from '@testing-library/react'; +import type { ComponentType, ReactNode } from 'react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { normalizeBase } from '../base-path'; +import type { SegmentParts } from '../location'; +import type { MosaicLocationOptions } from '../use-mosaic-location'; +import { useRouterScope } from '../use-router'; +import { useVirtualEntries, useVirtualWrite, VirtualStoreProvider } from '../virtual-store'; + +// The contract every location mode keeps towards the components above it. A mode plugs in a +// provider, a way to read an entry and a way to write one from outside, the way a host or a modal +// host would. Inherent differences between carriers are declared in `excludes`, so a difference is +// stated here rather than discovered in a review. +interface LocationMode { + name: string; + routing: MosaicLocationOptions['routing']; + Provider: ComponentType<{ initial?: Record; children: ReactNode }>; + useEntry: (base: string) => SegmentParts | null; + useOutsideWrite: () => (base: string, parts: SegmentParts | undefined) => void; + excludes: { + // A carrier that holds exactly one entry, such as the pathname, cannot host two bases at once. + severalBases?: boolean; + }; +} + +const virtual: LocationMode = { + name: 'virtual', + routing: 'virtual', + Provider: VirtualStoreProvider, + useEntry: base => useVirtualEntries().get(normalizeBase(base)) ?? null, + useOutsideWrite: useVirtualWrite, + excludes: {}, +}; + +const modes: LocationMode[] = [virtual]; + +const routes = { + index: '', + security: 'security', + statement: 'billing/statement/:statementId', +} as const; + +const otherRoutes = { + general: 'general', + members: 'members', +} as const; + +afterEach(() => cleanup()); + +describe.each(modes)('$name location', mode => { + describe.each([{ strict: false }, { strict: true }])('strict mode: $strict', ({ strict }) => { + beforeEach(() => configure({ reactStrictMode: strict })); + afterEach(() => configure({ reactStrictMode: false })); + + function Owner({ path = '/user', seen }: { path?: string; seen?: string[] }) { + const { route, go } = useRouterScope(routes, { routing: mode.routing, path, notFound: 'index' }); + seen?.push(route); + return ( + <> + {route} + + + ); + } + + function OtherOwner({ path = '/user' }: { path?: string }) { + const { route, go } = useRouterScope(otherRoutes, { routing: mode.routing, path, notFound: 'general' }); + return ( + <> + {route} + + + ); + } + + function Entry({ base = '/user' }: { base?: string }) { + return {JSON.stringify(mode.useEntry(base))}; + } + + function Outside({ base = '/user', parts }: { base?: string; parts: SegmentParts | undefined }) { + const write = mode.useOutsideWrite(); + return ( + + ); + } + + function Page({ initial, children }: { initial?: Record; children: ReactNode }) { + return {children}; + } + + const route = (path = '/user') => screen.getByTestId(`route${path}`).textContent; + const other = (path = '/user') => screen.getByTestId(`other${path}`).textContent; + const entry = (base = '/user') => JSON.parse(screen.getByTestId(`entry${base}`).textContent ?? 'null'); + const click = (name: string) => fireEvent.click(screen.getByRole('button', { name })); + + it('shows the root on a fresh mount and writes nothing', () => { + render( + + + + , + ); + expect(route()).toBe('index'); + expect(entry()).toBeNull(); + }); + + it('reflects a navigation in the route and the entry', () => { + render( + + + + , + ); + click('Go to security /user'); + expect(route()).toBe('security'); + expect(entry()).toEqual(['security']); + }); + + it('honours an entry written before mount on the first render', () => { + const seen: string[] = []; + render( + + + + , + ); + expect(seen[0]).toBe('security'); + expect(entry()).toEqual(['security']); + }); + + it('redirects an unknown segment to the fallback, every time it is written', () => { + const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); + render( + + + + + , + ); + expect(route()).toBe('index'); + expect(entry()).toEqual([]); + + click('Write /user ["nowhere"]'); + expect(route()).toBe('index'); + expect(entry()).toEqual([]); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + it('follows a write from outside to its own base and ignores other bases', () => { + render( + + + + + + , + ); + click('Write /organization ["members"]'); + expect(route()).toBe('index'); + + click('Write /user ["security"]'); + expect(route()).toBe('security'); + }); + + it('starts fresh after leaving and returning', () => { + const view = render( + + + + , + ); + click('Go to security /user'); + expect(route()).toBe('security'); + + view.rerender( + + + , + ); + view.rerender( + + + + , + ); + expect(route()).toBe('index'); + expect(entry()).toBeNull(); + }); + + it('drops an entry written before mount once the owner leaves', () => { + const view = render( + + + + , + ); + expect(route()).toBe('security'); + + view.rerender( + + + , + ); + expect(entry()).toBeNull(); + }); + + it('shows the root after an outside clear while mounted', () => { + render( + + + + + , + ); + expect(route()).toBe('security'); + + click('Write /user null'); + expect(route()).toBe('index'); + expect(entry()).toBeNull(); + }); + + // Needs React 19.2's Activity: a parked owner's effects stop and its state survives, and on + // resume it restores its segment unless the base was written meanwhile. React 18 has no way to + // produce that cycle, so this waits for a React 19 test project. + it.todo('comes back where it was after being parked and resumed'); + + it('lets an outside writer clear the entry after the owner navigated', () => { + render( + + + + + , + ); + click('Go to security /user'); + click('Write /user null'); + expect(route()).toBe('index'); + expect(entry()).toBeNull(); + }); + + it('shows a page written just before the owner is replaced in the same commit', () => { + function Replaceable({ generation }: { generation: number }) { + const write = mode.useOutsideWrite(); + return ( + <> + + + + ); + } + function Tree({ generation }: { generation: number }) { + return ( + + + + + ); + } + const view = render(); + click('Go to security /user'); + + act(() => { + fireEvent.click(screen.getByRole('button', { name: 'Write then replace' })); + view.rerender(); + }); + + expect(route()).toBe('security'); + expect(entry()).toEqual(['security']); + }); + + it('settles and warns when two owners of one base redirect each other', () => { + const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); + render( + + + + + , + ); + expect(route()).toBe('index'); + expect(other()).toBe('general'); + expect([[], ['general']]).toContainEqual(entry()); + expect(warn).toHaveBeenCalledTimes(1); + + click('Go to security /user'); + expect([['security'], ['general']]).toContainEqual(entry()); + warn.mockRestore(); + }); + + it.skipIf(mode.excludes.severalBases)('keeps owners of different bases independent', () => { + render( + + + + + + , + ); + click('Go to security /user'); + click('Go to members /organization'); + expect(route()).toBe('security'); + expect(other('/organization')).toBe('members'); + expect(entry()).toEqual(['security']); + expect(entry('/organization')).toEqual(['members']); + }); + }); +}); diff --git a/packages/mosaic/src/routing/__tests__/router.test.ts b/packages/mosaic/src/routing/__tests__/router.test.ts index 0c3c5f5eaac..150b6696bfb 100644 --- a/packages/mosaic/src/routing/__tests__/router.test.ts +++ b/packages/mosaic/src/routing/__tests__/router.test.ts @@ -19,6 +19,7 @@ function memoryLocation(initial: SegmentParts = []): MosaicLocation & { current: clear: () => { location.current = []; }, + own: () => () => {}, }; return location; } diff --git a/packages/mosaic/src/routing/__tests__/virtual-location.test.tsx b/packages/mosaic/src/routing/__tests__/virtual-location.test.tsx index df011edee47..468131519ff 100644 --- a/packages/mosaic/src/routing/__tests__/virtual-location.test.tsx +++ b/packages/mosaic/src/routing/__tests__/virtual-location.test.tsx @@ -1,11 +1,12 @@ -import { act, renderHook } from '@testing-library/react'; +import { act, render, renderHook, screen } from '@testing-library/react'; import type { ReactNode } from 'react'; +import { useEffect, useMemo } from 'react'; import { describe, expect, it } from 'vitest'; import { normalizeBase } from '../base-path'; import type { SegmentParts } from '../location'; import { createVirtualLocation } from '../virtual-location'; -import { useVirtualEntries, useVirtualWrite, VirtualStoreProvider } from '../virtual-store'; +import { useVirtualActions, useVirtualEntries, VirtualStoreProvider } from '../virtual-store'; describe('normalizeBase', () => { it('strips surrounding slashes and collapses the root', () => { @@ -23,7 +24,8 @@ function wrapperWith(initial?: Record) { } function useLocationAt(basePath: string) { - const location = createVirtualLocation(basePath, useVirtualWrite()); + const actions = useVirtualActions(); + const location = useMemo(() => createVirtualLocation(basePath, actions), [basePath, actions]); return { location, parts: location.useSegmentParts(), entries: useVirtualEntries() }; } @@ -35,9 +37,9 @@ describe('createVirtualLocation', () => { expect(result.current.parts).toEqual(['security']); }); - it('reads no parts when it has no entry', () => { + it('reads undefined when it has no entry', () => { const { result } = renderHook(() => useLocationAt('/user'), { wrapper: wrapperWith() }); - expect(result.current.parts).toEqual([]); + expect(result.current.parts).toBeUndefined(); }); it('writes its entry verbatim', () => { @@ -58,7 +60,7 @@ describe('createVirtualLocation', () => { result.current.location.clear(); }); - expect(result.current.parts).toEqual([]); + expect(result.current.parts).toBeUndefined(); expect(result.current.entries.has('/user')).toBe(false); }); @@ -76,7 +78,7 @@ describe('createVirtualLocation', () => { wrapper: wrapperWith({ '/user': ['security'] }), }); expect(result.current[0].parts).toEqual(['security']); - expect(result.current[1].parts).toEqual([]); + expect(result.current[1].parts).toBeUndefined(); act(() => { result.current[1].location.write(['members']); @@ -89,10 +91,81 @@ describe('createVirtualLocation', () => { result.current[0].location.clear(); }); - expect(result.current[0].parts).toEqual([]); + expect(result.current[0].parts).toBeUndefined(); expect(result.current[1].parts).toEqual(['members']); }); + describe('ownership', () => { + function Owner({ base }: { base: string }) { + const actions = useVirtualActions(); + const location = useMemo(() => createVirtualLocation(base, actions), [base, actions]); + useEffect(() => location.own(), [location]); + return null; + } + function Entries() { + return {JSON.stringify(Object.fromEntries(useVirtualEntries()))}; + } + const entries = () => JSON.parse(screen.getByTestId('entries').textContent ?? '{}'); + const seeded = { '/user': ['security'] }; + + it('drops an entry when its last owner leaves', () => { + const view = render( + + + + , + ); + expect(entries()).toEqual({ '/user': ['security'] }); + + view.rerender( + + + , + ); + expect(entries()).toEqual({}); + view.unmount(); + }); + + it('keeps an entry that never had an owner', () => { + const view = render( + + + + , + ); + view.rerender( + + + , + ); + expect(entries()).toEqual({ '/user': ['security'] }); + view.unmount(); + }); + + it('keeps an entry when one owner replaces another in the same commit', () => { + const view = render( + + + + , + ); + view.rerender( + + + + , + ); + expect(entries()).toEqual({ '/user': ['security'] }); + view.unmount(); + }); + }); + it('treats base spellings alike', () => { const { result } = renderHook(() => useLocationAt('user/'), { wrapper: wrapperWith({ '/user': ['security'] }) }); expect(result.current.parts).toEqual(['security']); diff --git a/packages/mosaic/src/routing/location.ts b/packages/mosaic/src/routing/location.ts index 29cca03a170..17ed6631256 100644 --- a/packages/mosaic/src/routing/location.ts +++ b/packages/mosaic/src/routing/location.ts @@ -8,11 +8,16 @@ export type SegmentParts = readonly string[]; // It also owns the encoding boundary. The parts it hands out are decoded exactly once, per part, as // close to the source as the source allows; the parts it is given are encoded on write the same way, // where the carrier has an encoded form at all. Nothing above a location encodes or decodes a route -// part again. `useSegmentParts` returns the same array for as long as the segment is unchanged, so -// callers can memoise on it. `clear` gives the entry up, for an owner such as a modal that only -// holds it while open. +// part again. +// +// `useSegmentParts` returns the same array for as long as the segment is unchanged, so callers can +// memoise on it, and `undefined` while the base has no entry at all: a modal reads that as closed, +// a router scope as the root. `clear` gives the entry up, for a modal host closing its modal. `own` +// counts a mounted owner in, and the function it returns counts it out again; an entry outlives its +// last owner no more than a URL segment outlives the host leaving the route. export interface MosaicLocation { - useSegmentParts: () => SegmentParts; + useSegmentParts: () => SegmentParts | undefined; write: (parts: SegmentParts, options?: WriteOptions) => void; clear: () => void; + own: () => () => void; } diff --git a/packages/mosaic/src/routing/routes.test-d.ts b/packages/mosaic/src/routing/routes.test-d.ts index 44b874684f4..6f276ca4273 100644 --- a/packages/mosaic/src/routing/routes.test-d.ts +++ b/packages/mosaic/src/routing/routes.test-d.ts @@ -17,7 +17,7 @@ describe('RouteParams', () => { }); describe('Router', () => { - const location: MosaicLocation = { useSegmentParts: () => [], write: () => {}, clear: () => {} }; + const location: MosaicLocation = { useSegmentParts: () => [], write: () => {}, clear: () => {}, own: () => () => {} }; const router = createRouter( { index: '', security: 'security', statement: 'billing/statement/:statementId' }, location, diff --git a/packages/mosaic/src/routing/use-mosaic-location.ts b/packages/mosaic/src/routing/use-mosaic-location.ts index c720ebc5d83..b0c6676a3e3 100644 --- a/packages/mosaic/src/routing/use-mosaic-location.ts +++ b/packages/mosaic/src/routing/use-mosaic-location.ts @@ -2,7 +2,7 @@ import { useMemo } from 'react'; import type { MosaicLocation } from './location'; import { createVirtualLocation } from './virtual-location'; -import { useVirtualWrite } from './virtual-store'; +import { useVirtualActions } from './virtual-store'; export type MosaicLocationOptions = { routing: 'virtual'; path?: string }; @@ -14,7 +14,7 @@ export interface RoutingProps { } export function useMosaicLocation(options: MosaicLocationOptions): MosaicLocation { - const write = useVirtualWrite(); + const actions = useVirtualActions(); const path = options.path ?? '/'; - return useMemo(() => createVirtualLocation(path, write), [path, write]); + return useMemo(() => createVirtualLocation(path, actions), [path, actions]); } diff --git a/packages/mosaic/src/routing/use-router.ts b/packages/mosaic/src/routing/use-router.ts index 3d10214189f..9148e626805 100644 --- a/packages/mosaic/src/routing/use-router.ts +++ b/packages/mosaic/src/routing/use-router.ts @@ -1,5 +1,5 @@ import { logger } from '@clerk/shared/logger'; -import { useEffect, useMemo, useState } from 'react'; +import { useEffect, useMemo, useRef, useState } from 'react'; import type { MosaicLocation, SegmentParts } from './location'; import type { Route, Router } from './router'; @@ -12,6 +12,7 @@ import type { MosaicLocationOptions } from './use-mosaic-location'; import { useMosaicLocation } from './use-mosaic-location'; const EMPTY_PARAMS: Record = Object.freeze({}); +const EMPTY_PARTS: SegmentParts = Object.freeze([]); const KEY_SEPARATOR = '\u0000'; @@ -59,6 +60,41 @@ function withRedirectLimit(location: MosaicLocation, path: string | undefined): }; } +// The scope owns its base while its effects are live. When the last owner leaves, the location +// drops the entry, so the next mount starts fresh the way a URL would after the host navigated +// away. Effects also stop when React parks the component without unmounting it (Activity), and +// then the component's state survives. In virtual mode the cleanup therefore parks the entry in +// state and the effects that run on resume write it back, unless something else wrote the base in +// between. The ownership effect's dependencies never change on a navigation, so its cleanup only +// runs on hide or unmount; it reads the entry to park from a ref that is written in an effect and +// read nowhere else. The parked value is consumed during render as soon as an entry is present +// again, so a mounted scope never falls back to it, and an entry cleared from outside shows the +// root. A URL mode holds the segment in its carrier and parks nothing. +function useOwnedSegment(location: MosaicLocation, entry: SegmentParts | undefined, parks: boolean): void { + const [parked, setParked] = useState(undefined); + if (parked !== undefined && entry !== undefined) { + setParked(undefined); + } + const latest = useRef(entry); + useEffect(() => { + latest.current = entry; + }, [entry]); + useEffect(() => { + if (parks && parked !== undefined && entry === undefined) { + location.write(parked, { replace: true }); + } + }, [location, parks, parked, entry]); + useEffect(() => { + const release = location.own(); + return () => { + if (parks) { + setParked(latest.current); + } + release(); + }; + }, [location, parks]); +} + function missingScope(): never { throw new Error('[clerk] useRouter needs a above it; the segment owner calls useRouterScope.'); } @@ -67,6 +103,7 @@ interface Routing { location: MosaicLocation; table: Routes; router: Router; + entry: SegmentParts | undefined; parts: SegmentParts; matched: Route | undefined; } @@ -75,9 +112,10 @@ function useRouting(routes: Routes, options: MosaicLo const location = useMosaicLocation(options); const table = useStableTable(routes); const router = useMemo(() => createRouter(table, location), [table, location]); - const parts = location.useSegmentParts(); + const entry = location.useSegmentParts(); + const parts = entry ?? EMPTY_PARTS; const matched = useMemo(() => router.parse(parts), [router, parts]); - return { location, table, router, parts, matched }; + return { location, table, router, entry, parts, matched }; } export interface RouterScopeOptions { @@ -102,7 +140,7 @@ export function useRouterScope( { routing, path, notFound }: RouterScopeOptions, ): RouterScope { const scope = useMemo(() => createRoutingScope({ routing, path }), [routing, path]); - const { location, table, router, parts, matched } = useRouting(routes, { routing, path }); + const { location, table, router, entry, parts, matched } = useRouting(routes, { routing, path }); const current = useMemo>( () => matched ?? { name: notFound, params: EMPTY_PARAMS }, [matched, notFound], @@ -113,6 +151,8 @@ export function useRouterScope( return router.parse(target) ? target : undefined; }, [table, router, notFound]); + useOwnedSegment(location, entry, routing === 'virtual'); + const redirect = useMemo(() => withRedirectLimit(location, path), [location, path]); const redirectTo = matched === undefined ? notFound : undefined; useEffect(() => { diff --git a/packages/mosaic/src/routing/virtual-location.ts b/packages/mosaic/src/routing/virtual-location.ts index 42c41a4989f..95e39a381fe 100644 --- a/packages/mosaic/src/routing/virtual-location.ts +++ b/packages/mosaic/src/routing/virtual-location.ts @@ -1,14 +1,15 @@ import { normalizeBase } from './base-path'; import type { MosaicLocation } from './location'; -import { useVirtualSegment, type VirtualWrite } from './virtual-store'; +import { useVirtualSegment, type VirtualActions } from './virtual-store'; // One entry of the page's virtual store, read and written as decoded parts. Nothing to encode: // virtual state has no URL form until a carrier projects it. -export function createVirtualLocation(basePath: string, write: VirtualWrite): MosaicLocation { +export function createVirtualLocation(basePath: string, actions: VirtualActions): MosaicLocation { const base = normalizeBase(basePath); return { useSegmentParts: () => useVirtualSegment(base), - write: parts => write(base, parts), - clear: () => write(base, undefined), + write: parts => actions.write(base, parts), + clear: () => actions.write(base, undefined), + own: () => actions.own(base), }; } diff --git a/packages/mosaic/src/routing/virtual-store.tsx b/packages/mosaic/src/routing/virtual-store.tsx index c4e874a2d76..d2f987aee6f 100644 --- a/packages/mosaic/src/routing/virtual-store.tsx +++ b/packages/mosaic/src/routing/virtual-store.tsx @@ -1,7 +1,7 @@ 'use client'; import type { ReactNode } from 'react'; -import { createContext, useCallback, useContext, useState } from 'react'; +import { createContext, useContext, useEffect, useRef, useState } from 'react'; import { normalizeBase } from './base-path'; import type { SegmentParts } from './location'; @@ -20,16 +20,27 @@ import type { SegmentParts } from './location'; We only need the external behavior to match the other modes however, so internally we keep state in a map instead of trying to cram it into a single "url"-like string. Nothing is encoded since it never goes into a real url. + + An entry lives for as long as its base has an owner, the way a URL segment lives for as long as + the host is on that route. A mounted router scope counts itself in with `own`; when the last + owner of a base leaves, the sweep below drops the entry. Only a base that has had an owner is + swept, so an entry written ahead of a later mount survives until that owner has come and gone. + Counting in store state, and sweeping after the commit, is what makes an owner replaced in the + same commit (a modal reopened mid-exit, StrictMode's doubled effects) keep its entry: the count + never reaches zero in a committed state. */ export type VirtualEntries = ReadonlyMap; export type VirtualWrite = (base: string, parts: SegmentParts | undefined) => void; -const EMPTY_PARTS: SegmentParts = Object.freeze([]); +export interface VirtualActions { + write: VirtualWrite; + own: (base: string) => () => void; +} const VirtualEntriesContext = createContext(undefined); -const VirtualWriteContext = createContext(undefined); +const VirtualActionsContext = createContext(undefined); export interface VirtualStoreProviderProps { initial?: Record; @@ -40,22 +51,50 @@ export function VirtualStoreProvider({ initial, children }: VirtualStoreProvider const [entries, setEntries] = useState( () => new Map(Object.entries(initial ?? {}).map(([base, parts]) => [normalizeBase(base), parts])), ); - const write = useCallback((base, parts) => { - setEntries(previous => { - const next = new Map(previous); - if (parts === undefined) { - next.delete(base); - } else { - next.set(base, parts); + const [owners, setOwners] = useState>(() => new Map()); + const [actions] = useState(() => { + const write: VirtualWrite = (base, parts) => { + setEntries(previous => { + if (parts === undefined && !previous.has(base)) { + return previous; + } + const next = new Map(previous); + if (parts === undefined) { + next.delete(base); + } else { + next.set(base, parts); + } + return next; + }); + }; + const count = (base: string, delta: number) => { + setOwners(previous => new Map(previous).set(base, (previous.get(base) ?? 0) + delta)); + }; + return { + write, + own: base => { + const key = normalizeBase(base); + count(key, 1); + return () => count(key, -1); + }, + }; + }); + + const previousOwners = useRef(owners); + useEffect(() => { + const previous = previousOwners.current; + previousOwners.current = owners; + for (const [base, had] of previous) { + if (had > 0 && (owners.get(base) ?? 0) === 0) { + actions.write(base, undefined); } - return next; - }); - }, []); + } + }, [owners, actions]); return ( - + {children} - + ); } @@ -67,12 +106,16 @@ export function useVirtualEntries(): VirtualEntries { return useContext(VirtualEntriesContext) ?? missingProvider(); } +export function useVirtualActions(): VirtualActions { + return useContext(VirtualActionsContext) ?? missingProvider(); +} + export function useVirtualWrite(): VirtualWrite { - return useContext(VirtualWriteContext) ?? missingProvider(); + return useVirtualActions().write; } -// The entry for a base, or no parts. The array comes straight from the map, so it keeps its -// identity until that entry is written again. -export function useVirtualSegment(base: string): SegmentParts { - return useVirtualEntries().get(normalizeBase(base)) ?? EMPTY_PARTS; +// The entry for a base, or undefined when the base has none. The array comes straight from the map, +// so it keeps its identity until that entry is written again. +export function useVirtualSegment(base: string): SegmentParts | undefined { + return useVirtualEntries().get(normalizeBase(base)); } From 91604ae11eef02e39e1f49d0347538b5746b4094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Fri, 6 Nov 2026 14:25:12 +0100 Subject: [PATCH 15/26] Tweak parking mechanism --- .../__tests__/location-contract.test.tsx | 20 ++++++ packages/mosaic/src/routing/use-router.ts | 68 ++++++++++++------- 2 files changed, 63 insertions(+), 25 deletions(-) diff --git a/packages/mosaic/src/routing/__tests__/location-contract.test.tsx b/packages/mosaic/src/routing/__tests__/location-contract.test.tsx index b49a1066b9a..48feb51a4c4 100644 --- a/packages/mosaic/src/routing/__tests__/location-contract.test.tsx +++ b/packages/mosaic/src/routing/__tests__/location-contract.test.tsx @@ -212,6 +212,26 @@ describe.each(modes)('$name location', mode => { expect(entry()).toBeNull(); }); + it('starts on the root after moving to another base, leaving nothing behind', () => { + function Tree({ path }: { path: string }) { + return ( + + + + + + ); + } + const view = render(); + click('Go to security /user'); + expect(route()).toBe('security'); + + view.rerender(); + expect(route('/account')).toBe('index'); + expect(entry('/account')).toBeNull(); + expect(entry()).toBeNull(); + }); + it('shows the root after an outside clear while mounted', () => { render( diff --git a/packages/mosaic/src/routing/use-router.ts b/packages/mosaic/src/routing/use-router.ts index 9148e626805..e21e4aa140e 100644 --- a/packages/mosaic/src/routing/use-router.ts +++ b/packages/mosaic/src/routing/use-router.ts @@ -60,39 +60,54 @@ function withRedirectLimit(location: MosaicLocation, path: string | undefined): }; } -// The scope owns its base while its effects are live. When the last owner leaves, the location -// drops the entry, so the next mount starts fresh the way a URL would after the host navigated -// away. Effects also stop when React parks the component without unmounting it (Activity), and -// then the component's state survives. In virtual mode the cleanup therefore parks the entry in -// state and the effects that run on resume write it back, unless something else wrote the base in -// between. The ownership effect's dependencies never change on a navigation, so its cleanup only -// runs on hide or unmount; it reads the entry to park from a ref that is written in an effect and -// read nowhere else. The parked value is consumed during render as soon as an entry is present -// again, so a mounted scope never falls back to it, and an entry cleared from outside shows the -// root. A URL mode holds the segment in its carrier and parks nothing. -function useOwnedSegment(location: MosaicLocation, entry: SegmentParts | undefined, parks: boolean): void { - const [parked, setParked] = useState(undefined); - if (parked !== undefined && entry !== undefined) { +// The scope owns its base while its effects are live, and returns the entry it should render from. +// When the last owner leaves, the location drops the entry, so the next mount starts fresh the way +// a URL would after the host navigated away. Effects also stop when React parks the component +// without unmounting it (Activity), and then the component's state survives. In virtual mode the +// cleanup therefore parks the entry in state, the scope keeps rendering from the parked entry while +// it is hidden, so the page's subtree keeps its state, and the effects that run on resume write it +// back, unless something else wrote the base in between. The ownership effect's dependencies never +// change on a navigation, so its cleanup only runs on hide, unmount or a change of location; it +// reads the entry to park from a ref that is written in an effect and read nowhere else. A park +// is tied to the location it was taken from, so a scope that moves to another base does not carry +// its page along. The parked value is consumed during render as soon as an entry is present again, +// so a mounted scope never falls back to it, and an entry cleared from outside shows the root. A +// URL mode holds the segment in its carrier and parks nothing. +interface ParkedSegment { + location: MosaicLocation; + parts: SegmentParts; +} + +function useOwnedSegment( + location: MosaicLocation, + entry: SegmentParts | undefined, + parks: boolean, +): SegmentParts | undefined { + const [parked, setParked] = useState(undefined); + if (parked !== undefined && (entry !== undefined || parked.location !== location)) { setParked(undefined); } + const held = parked !== undefined && parked.location === location ? parked.parts : undefined; const latest = useRef(entry); useEffect(() => { latest.current = entry; }, [entry]); useEffect(() => { - if (parks && parked !== undefined && entry === undefined) { - location.write(parked, { replace: true }); + if (parks && held !== undefined && entry === undefined) { + location.write(held, { replace: true }); } - }, [location, parks, parked, entry]); + }, [location, parks, held, entry]); useEffect(() => { const release = location.own(); return () => { - if (parks) { - setParked(latest.current); + const parts = latest.current; + if (parks && parts !== undefined) { + setParked({ location, parts }); } release(); }; }, [location, parks]); + return entry ?? held; } function missingScope(): never { @@ -104,8 +119,6 @@ interface Routing { table: Routes; router: Router; entry: SegmentParts | undefined; - parts: SegmentParts; - matched: Route | undefined; } function useRouting(routes: Routes, options: MosaicLocationOptions): Routing { @@ -113,9 +126,13 @@ function useRouting(routes: Routes, options: MosaicLo const table = useStableTable(routes); const router = useMemo(() => createRouter(table, location), [table, location]); const entry = location.useSegmentParts(); + return { location, table, router, entry }; +} + +function useMatch(router: Router, entry: SegmentParts | undefined) { const parts = entry ?? EMPTY_PARTS; const matched = useMemo(() => router.parse(parts), [router, parts]); - return { location, table, router, entry, parts, matched }; + return { parts, matched }; } export interface RouterScopeOptions { @@ -140,7 +157,9 @@ export function useRouterScope( { routing, path, notFound }: RouterScopeOptions, ): RouterScope { const scope = useMemo(() => createRoutingScope({ routing, path }), [routing, path]); - const { location, table, router, entry, parts, matched } = useRouting(routes, { routing, path }); + const { location, table, router, entry } = useRouting(routes, { routing, path }); + const held = useOwnedSegment(location, entry, routing === 'virtual'); + const { parts, matched } = useMatch(router, held); const current = useMemo>( () => matched ?? { name: notFound, params: EMPTY_PARAMS }, [matched, notFound], @@ -151,8 +170,6 @@ export function useRouterScope( return router.parse(target) ? target : undefined; }, [table, router, notFound]); - useOwnedSegment(location, entry, routing === 'virtual'); - const redirect = useMemo(() => withRedirectLimit(location, path), [location, path]); const redirectTo = matched === undefined ? notFound : undefined; useEffect(() => { @@ -185,7 +202,8 @@ export interface NestedRouter { // For anything rendered inside a segment owner. It matches its own table against the segment and // reports no route when the segment belongs to a sibling; it never redirects. export function useRouter(routes: Routes): NestedRouter { - const { router, matched } = useRouting(routes, useInheritedScope() ?? missingScope()); + const { router, entry } = useRouting(routes, useInheritedScope() ?? missingScope()); + const { matched } = useMatch(router, entry); return useMemo( () => ({ route: matched?.name, params: matched?.params ?? EMPTY_PARAMS, go: router.go }), From 219e0f8e02400c5c5304b7e52fc750a4353366c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Fri, 6 Nov 2026 14:51:02 +0100 Subject: [PATCH 16/26] Tighten routable paths check --- .../organization-profile.layout.test.tsx | 10 +++++ .../organization-profile.layout.ts | 2 +- .../profile-pages/profile-pages.layout.ts | 42 +++++++++++++------ .../__tests__/user-profile.layout.test.ts | 17 ++++++++ .../user-profile/user-profile.layout.ts | 2 +- .../src/routing/__tests__/router.test.ts | 13 ++++++ .../src/routing/__tests__/routes.test.ts | 21 +++------- packages/mosaic/src/routing/router.ts | 9 +++- packages/mosaic/src/routing/routes.ts | 16 +++---- packages/mosaic/src/routing/virtual-store.tsx | 15 ++++++- 10 files changed, 106 insertions(+), 41 deletions(-) diff --git a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.layout.test.tsx b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.layout.test.tsx index 289f19ff8ce..599aec6e969 100644 --- a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.layout.test.tsx +++ b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile.layout.test.tsx @@ -17,6 +17,16 @@ describe('resolveOrganizationProfilePages', () => { }); }); +describe('resolveOrganizationProfilePages custom paths', () => { + it("accepts a path that is only a built-in segment in the user profile's combined view", () => { + const keys: CustomOrganizationProfilePage = { label: 'Keys', path: 'organization/api-keys', content: null }; + expect(resolveOrganizationProfilePages(['general'], [keys])).toEqual([ + { id: 'general' }, + { id: 'organization/api-keys', custom: keys }, + ]); + }); +}); + describe('getAvailableOrganizationProfilePages', () => { it('lists api keys given content', () => { expect(getAvailableOrganizationProfilePages({ general, apiKeys:

API keys

})).toEqual(['general', 'apiKeys']); diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.layout.ts b/packages/mosaic/src/features/organization-profile/organization-profile.layout.ts index 1c910312772..3848e387a76 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.layout.ts +++ b/packages/mosaic/src/features/organization-profile/organization-profile.layout.ts @@ -45,7 +45,7 @@ export function resolveOrganizationProfilePages( customPages: readonly CustomOrganizationProfilePage[] = [], order?: readonly string[], ): OrganizationProfileNavEntry[] { - const uniqueCustomPages = withUniquePaths(withRoutablePaths(customPages)); + const uniqueCustomPages = withUniquePaths(withRoutablePaths(customPages, ORGANIZATION_PROFILE_PAGE_IDS)); const customById = new Map(uniqueCustomPages.map(page => [page.path, page])); const builtInIds = new Set(builtIn); const entries: OrganizationProfileNavEntry[] = [ diff --git a/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts b/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts index c384566d88c..b591a6c3632 100644 --- a/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts +++ b/packages/mosaic/src/features/profile-pages/profile-pages.layout.ts @@ -1,7 +1,5 @@ import { logger } from '@clerk/shared/logger'; -import { isLiteralPattern } from '../../routing/routes'; - const BUILT_IN_SEGMENTS: Readonly> = { apiKeys: 'api-keys', 'organization/apiKeys': 'organization/api-keys', @@ -15,24 +13,44 @@ export function pageParts(page: string | undefined): string[] { return page === undefined ? [] : pageSegment(page).split('/').filter(Boolean); } -const BUILT_IN_SEGMENT_IDS = new Map(Object.entries(BUILT_IN_SEGMENTS).map(([id, segment]) => [segment, id])); - -export function withRoutablePaths(pages: readonly Page[]): Page[] { +// A custom page's path becomes a route literal and is compared raw elsewhere, so it has to be +// exactly its own segment: decoded, without a leading colon, and without the slashes the codec +// would strip. +export function withRoutablePaths( + pages: readonly Page[], + builtInIds: readonly string[], +): Page[] { + const shadowed = new Map(builtInIds.filter(id => pageSegment(id) !== id).map(id => [pageSegment(id), id])); return pages.filter(page => { - const builtIn = BUILT_IN_SEGMENT_IDS.get(page.path); + const builtIn = shadowed.get(page.path); if (builtIn !== undefined) { logger.warnOnce( `[Clerk] Custom page path "${page.path}" is the segment of the built-in "${builtIn}" page, so the page is left out. Use "${builtIn}" as the path to replace that page.`, ); return false; } - if (isLiteralPattern(page.path)) { - return true; + if (page.path === '') { + logger.warnOnce('[Clerk] A custom page needs a path, so a page without one is left out.'); + return false; + } + const parts = pageParts(page.path); + if (parts.join('/') !== page.path) { + logger.warnOnce( + `[Clerk] Custom page path "${page.path}" has a leading, trailing or doubled slash, so the page is left out. Use "${parts.join('/')}".`, + ); + return false; + } + if (parts.some(part => part.startsWith(':'))) { + logger.warnOnce(`[Clerk] Custom page path "${page.path}" cannot start with a colon, so the page is left out.`); + return false; } - logger.warnOnce( - `[Clerk] Custom page path "${page.path}" cannot be a route, so the page is left out. Write it decoded, without a leading colon.`, - ); - return false; + if (parts.some(part => part.includes('%'))) { + logger.warnOnce( + `[Clerk] Custom page path "${page.path}" contains percent-encoding, so the page is left out. Write it as plain text.`, + ); + return false; + } + return true; }); } diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts b/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts index 08fd1ba2a3f..12f47ce7ac0 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile.layout.test.ts @@ -67,6 +67,23 @@ describe('resolveUserProfilePages', () => { warn.mockRestore(); }); + it('leaves out a custom page whose path is not written as its segment, and warns', () => { + const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); + const trailing: CustomProfilePage = { label: 'Security', path: 'security/', content: null }; + const leading: CustomProfilePage = { label: 'Terms', path: '/terms', content: null }; + const empty: CustomProfilePage = { label: 'Empty', path: '', content: null }; + + expect(resolveUserProfilePages(['account', 'security'], [trailing, leading, empty, help])).toEqual([ + { id: 'account' }, + { id: 'security' }, + { id: 'help', custom: help }, + ]); + expect(warn).toHaveBeenCalledTimes(3); + expect(warn.mock.calls[0][0]).toContain('slash'); + expect(warn.mock.calls[0][0]).toContain('Use "security"'); + warn.mockRestore(); + }); + it('lists the built-ins, then the custom pages by path', () => { expect(resolveUserProfilePages(['account', 'security'], [terms, help])).toEqual([ { id: 'account' }, diff --git a/packages/mosaic/src/features/user-profile/user-profile.layout.ts b/packages/mosaic/src/features/user-profile/user-profile.layout.ts index fa625790efe..7d905119ddc 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.layout.ts +++ b/packages/mosaic/src/features/user-profile/user-profile.layout.ts @@ -47,7 +47,7 @@ export function resolveUserProfilePages( customPages: readonly CustomProfilePage[] = [], order?: readonly string[], ): UserProfileNavEntry[] { - const uniqueCustomPages = withUniquePaths(withRoutablePaths(customPages)); + const uniqueCustomPages = withUniquePaths(withRoutablePaths(customPages, USER_PROFILE_PAGE_IDS)); const customById = new Map(uniqueCustomPages.map(page => [page.path, page])); const builtInIds = new Set(builtIn); const entries: UserProfileNavEntry[] = [ diff --git a/packages/mosaic/src/routing/__tests__/router.test.ts b/packages/mosaic/src/routing/__tests__/router.test.ts index 150b6696bfb..b33e98b4d47 100644 --- a/packages/mosaic/src/routing/__tests__/router.test.ts +++ b/packages/mosaic/src/routing/__tests__/router.test.ts @@ -1,3 +1,4 @@ +import { logger } from '@clerk/shared/logger'; import { describe, expect, it, vi } from 'vitest'; import type { MosaicLocation, SegmentParts } from '../location'; @@ -52,6 +53,18 @@ describe('createRouter go', () => { expect(location.current).toEqual(['billing', 'statement', 'st 1']); }); + it('ignores an unknown route name and warns', () => { + const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); + const location = memoryLocation(['account']); + const router = createRouter(routes as Record, location); + + router.go('nowhere'); + + expect(location.current).toEqual(['account']); + expect(warn).toHaveBeenCalledTimes(1); + warn.mockRestore(); + }); + it('passes write options through', () => { const location = memoryLocation(['account']); const write = vi.spyOn(location, 'write'); diff --git a/packages/mosaic/src/routing/__tests__/routes.test.ts b/packages/mosaic/src/routing/__tests__/routes.test.ts index dd50f7c0ee4..3001716808a 100644 --- a/packages/mosaic/src/routing/__tests__/routes.test.ts +++ b/packages/mosaic/src/routing/__tests__/routes.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { buildParts, compilePattern, compileRoutes, isLiteralPattern, matchRoute, routeParts } from '../routes'; +import { buildParts, compilePattern, compileRoutes, matchRoute, routeParts } from '../routes'; const routes = compileRoutes({ index: '', @@ -70,22 +70,11 @@ describe('compilePattern', () => { expect(compilePattern('api keys')).toEqual([{ param: false, value: 'api keys' }]); expect(() => compilePattern('api%20keys')).toThrow('written decoded'); }); -}); - -describe('isLiteralPattern', () => { - it('accepts decoded parts', () => { - expect(isLiteralPattern('api-keys')).toBe(true); - expect(isLiteralPattern('organization/general')).toBe(true); - expect(isLiteralPattern('my page')).toBe(true); - expect(isLiteralPattern('café')).toBe(true); - }); - it('rejects an empty pattern, a param, and an encoded literal', () => { - expect(isLiteralPattern('')).toBe(false); - expect(isLiteralPattern('/')).toBe(false); - expect(isLiteralPattern(':id')).toBe(false); - expect(isLiteralPattern('terms/:id?')).toBe(false); - expect(isLiteralPattern('my%20page')).toBe(false); + it('rejects a required part after an optional one', () => { + expect(() => compilePattern('a/:b?/c')).toThrow('optional parts come last'); + expect(() => compilePattern('a/:b?/:c')).toThrow('optional parts come last'); + expect(compilePattern('a/:b?/:c?')).toHaveLength(3); }); }); diff --git a/packages/mosaic/src/routing/router.ts b/packages/mosaic/src/routing/router.ts index de73fa17646..3085b9c80a6 100644 --- a/packages/mosaic/src/routing/router.ts +++ b/packages/mosaic/src/routing/router.ts @@ -1,3 +1,5 @@ +import { logger } from '@clerk/shared/logger'; + import type { MosaicLocation, SegmentParts, WriteOptions } from './location'; import { buildParts, @@ -37,8 +39,13 @@ export function createRouter( return match ? { name: match.name, params: match.params } : undefined; }, go: (name, ...args) => { + const pattern = patternsByName.get(name); + if (pattern === undefined) { + logger.warnOnce(`[Clerk] "${name}" is not a route of this router, so nothing was navigated to.`); + return; + } const [params = {}, options] = args; - location.write(buildParts(patternsByName.get(name) ?? [], params), options); + location.write(buildParts(pattern, params), options); }, }; } diff --git a/packages/mosaic/src/routing/routes.ts b/packages/mosaic/src/routing/routes.ts index 1a7075dafb0..f989cb61ca8 100644 --- a/packages/mosaic/src/routing/routes.ts +++ b/packages/mosaic/src/routing/routes.ts @@ -63,16 +63,9 @@ function assertDecodedLiteral(part: string): void { } } -// 'api-keys' -> true 'my page' -> true 'my%20page' -> false ':id' -> false '' -> false -// Whether a string from outside, such as a custom page path, can be used as a literal route as is. -export function isLiteralPattern(pattern: string): boolean { - const parts = partsOf(pattern); - return parts.length > 0 && parts.every(part => !part.startsWith(':') && isDecodedLiteral(part)); -} - // 'billing/plans/:planId?' -> [{ param: false, value: 'billing' }, { param: false, value: 'plans' }, { param: true, name: 'planId', optional: true }] export function compilePattern(pattern: string): PatternPart[] { - return partsOf(pattern).map(part => { + const parts = partsOf(pattern).map(part => { if (!part.startsWith(':')) { assertDecodedLiteral(part); return { param: false, value: part }; @@ -80,6 +73,13 @@ export function compilePattern(pattern: string): PatternPart[] { const optional = part.endsWith('?'); return { param: true, name: part.slice(1, optional ? -1 : undefined), optional }; }); + const firstOptional = parts.findIndex(part => part.param && part.optional); + if (firstOptional !== -1 && parts.slice(firstOptional).some(part => !part.param || !part.optional)) { + throw new Error( + `[clerk] Route pattern "${pattern}" has a required part after an optional one; optional parts come last.`, + ); + } + return parts; } // 'billing/plans/:planId?' -> 2 'billing/statement/:id' -> 3 diff --git a/packages/mosaic/src/routing/virtual-store.tsx b/packages/mosaic/src/routing/virtual-store.tsx index d2f987aee6f..cc6bbaa6d9e 100644 --- a/packages/mosaic/src/routing/virtual-store.tsx +++ b/packages/mosaic/src/routing/virtual-store.tsx @@ -80,16 +80,27 @@ export function VirtualStoreProvider({ initial, children }: VirtualStoreProvider }; }); + // The sweep runs after the commit in which a count reached zero. A write can land in between, + // when a click opens a modal before the passive effects of the unmount commit have flushed, so + // it only deletes the entry that was committed with the zero count, never a newer one. const previousOwners = useRef(owners); useEffect(() => { const previous = previousOwners.current; previousOwners.current = owners; for (const [base, had] of previous) { if (had > 0 && (owners.get(base) ?? 0) === 0) { - actions.write(base, undefined); + const expected = entries.get(base); + setEntries(current => { + if (expected === undefined || current.get(base) !== expected) { + return current; + } + const next = new Map(current); + next.delete(base); + return next; + }); } } - }, [owners, actions]); + }, [owners, entries]); return ( From d11974481b9eaa31c83061501980002ecad83d46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Fri, 6 Nov 2026 15:25:36 +0100 Subject: [PATCH 17/26] Add root option to useRouterScope and clean up unnecessary code --- .../profile-pages.controller.test.tsx | 16 ++++ .../profile-pages/profile-pages.controller.ts | 9 +-- packages/mosaic/src/modals/use-modal.test.tsx | 4 +- .../__tests__/location-contract.test.tsx | 12 +-- .../src/routing/__tests__/use-router.test.tsx | 63 ++++++++-------- packages/mosaic/src/routing/use-router.ts | 73 ++++--------------- 6 files changed, 72 insertions(+), 105 deletions(-) diff --git a/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx b/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx index 4fff03d939c..58ecc43c999 100644 --- a/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx +++ b/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx @@ -35,4 +35,20 @@ describe('useProfilePagesController', () => { expect(result.current.pages.activePage).toBe('general'); expect(result.current.segment).toBeUndefined(); }); + + it("canonicalises the root page's own segment to the base", () => { + const { result } = renderHook( + () => ({ + pages: useProfilePagesController(['general', 'members'], { routing: 'virtual', path: '/organization' }), + segment: useVirtualSegment('/organization'), + }), + { + wrapper: ({ children }) => ( + {children} + ), + }, + ); + expect(result.current.pages.activePage).toBe('general'); + expect(result.current.segment).toEqual([]); + }); }); diff --git a/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts b/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts index 1797480694c..c190a37eb64 100644 --- a/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts +++ b/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts @@ -5,11 +5,10 @@ import { pageSegment } from './profile-pages.layout'; const NO_PAGE = ''; export function useProfilePagesController(pageIds: readonly string[], options: MosaicLocationOptions) { - const rootPage = pageIds[0] ?? NO_PAGE; - const { route, go } = useRouterScope( - Object.fromEntries(pageIds.map(id => [id, id === rootPage ? '' : pageSegment(id)])), - { ...options, notFound: rootPage }, - ); + const { route, go } = useRouterScope(Object.fromEntries(pageIds.map(id => [id, pageSegment(id)])), { + ...options, + root: pageIds[0] ?? NO_PAGE, + }); return { activePage: route, onPageChange: go }; } diff --git a/packages/mosaic/src/modals/use-modal.test.tsx b/packages/mosaic/src/modals/use-modal.test.tsx index 93ea682f130..e137a926e7c 100644 --- a/packages/mosaic/src/modals/use-modal.test.tsx +++ b/packages/mosaic/src/modals/use-modal.test.tsx @@ -532,7 +532,7 @@ describe('a modal surface that routes', () => { const pages = { account: 'account', security: 'security' } as const; function RoutingContent({ path }: ModalContentProps) { - const { route } = useRouterScope(pages, { routing: 'virtual', path, notFound: 'account' }); + const { route } = useRouterScope(pages, { routing: 'virtual', path, root: 'account' }); return {route}; } @@ -601,7 +601,7 @@ describe('a modal surface that routes', () => { act(() => ref.handle?.open()); await flush(); - expect(entry()).toEqual(['account']); + expect(entry()).toEqual([]); act(() => ref.handle?.close()); await flush(); act(() => ref.handle?.open({ page: 'security' })); diff --git a/packages/mosaic/src/routing/__tests__/location-contract.test.tsx b/packages/mosaic/src/routing/__tests__/location-contract.test.tsx index 48feb51a4c4..095954863f3 100644 --- a/packages/mosaic/src/routing/__tests__/location-contract.test.tsx +++ b/packages/mosaic/src/routing/__tests__/location-contract.test.tsx @@ -55,7 +55,7 @@ describe.each(modes)('$name location', mode => { afterEach(() => configure({ reactStrictMode: false })); function Owner({ path = '/user', seen }: { path?: string; seen?: string[] }) { - const { route, go } = useRouterScope(routes, { routing: mode.routing, path, notFound: 'index' }); + const { route, go } = useRouterScope(routes, { routing: mode.routing, path, root: 'index' }); seen?.push(route); return ( <> @@ -66,7 +66,7 @@ describe.each(modes)('$name location', mode => { } function OtherOwner({ path = '/user' }: { path?: string }) { - const { route, go } = useRouterScope(otherRoutes, { routing: mode.routing, path, notFound: 'general' }); + const { route, go } = useRouterScope(otherRoutes, { routing: mode.routing, path, root: 'general' }); return ( <> {route} @@ -296,7 +296,7 @@ describe.each(modes)('$name location', mode => { expect(entry()).toEqual(['security']); }); - it('settles and warns when two owners of one base redirect each other', () => { + it('lets two owners of one base agree on the base, so neither can loop the other', () => { const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); render( @@ -307,11 +307,11 @@ describe.each(modes)('$name location', mode => { ); expect(route()).toBe('index'); expect(other()).toBe('general'); - expect([[], ['general']]).toContainEqual(entry()); - expect(warn).toHaveBeenCalledTimes(1); + expect(entry()).toEqual([]); click('Go to security /user'); - expect([['security'], ['general']]).toContainEqual(entry()); + expect(entry()).toEqual([]); + expect(warn).not.toHaveBeenCalled(); warn.mockRestore(); }); diff --git a/packages/mosaic/src/routing/__tests__/use-router.test.tsx b/packages/mosaic/src/routing/__tests__/use-router.test.tsx index a43c80e9e15..17378192f48 100644 --- a/packages/mosaic/src/routing/__tests__/use-router.test.tsx +++ b/packages/mosaic/src/routing/__tests__/use-router.test.tsx @@ -16,7 +16,7 @@ const routes = { statement: 'billing/statement/:statementId', } as const; -const userScope: RouterScopeOptions = { routing: 'virtual', path: '/user', notFound: 'account' }; +const userScope: RouterScopeOptions = { routing: 'virtual', path: '/user', root: 'account' }; function ScopeOf({ options, children }: { options: RouterScopeOptions; children: ReactNode }) { const { scope } = useRouterScope(routes, options); @@ -68,20 +68,33 @@ describe('useRouterScope', () => { expect(result.current.params).toEqual({ statementId: 'st/1 %2F' }); }); - it('shows the not-found route at once and redirects its entry to it', () => { + it('shows the root for an unknown segment at once and replaces the entry with the base', () => { const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), segment: useUserSegment() }), { wrapper: wrapperWith({ '/user': ['nowhere'] }), }); expect(result.current.router.route).toBe('account'); - expect(result.current.segment).toEqual(['account']); + expect(result.current.segment).toEqual([]); }); - it('redirects an empty entry to the not-found route', () => { + it('shows the root for an absent entry without writing', () => { const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), segment: useUserSegment() }), { wrapper: wrapperWith(), }); expect(result.current.router.route).toBe('account'); - expect(result.current.segment).toEqual(['account']); + expect(result.current.segment).toBeUndefined(); + }); + + it("canonicalises the root's own segment to the base", () => { + const { result } = renderHook(() => ({ router: useRouterScope(routes, userScope), segment: useUserSegment() }), { + wrapper: wrapperWith({ '/user': ['account'] }), + }); + expect(result.current.router.route).toBe('account'); + expect(result.current.segment).toEqual([]); + + act(() => { + result.current.router.go('account'); + }); + expect(result.current.segment).toEqual([]); }); it('follows a write made to its entry by someone else', () => { @@ -111,7 +124,7 @@ describe('useRouterScope', () => { const { result } = renderHook( () => ({ user: useRouterScope(routes, userScope), - org: useRouterScope(orgRoutes, { routing: 'virtual', path: '/organization', notFound: 'general' }), + org: useRouterScope(orgRoutes, { routing: 'virtual', path: '/organization', root: 'general' }), }), { wrapper: wrapperWith() }, ); @@ -135,14 +148,14 @@ describe('useRouterScope', () => { () => ({ router: useRouterScope(routes, userScope), segment: useUserSegment(), write: useVirtualWrite() }), { wrapper: wrapperWith({ '/user': ['nowhere'] }) }, ); - expect(result.current.segment).toEqual(['account']); + expect(result.current.segment).toEqual([]); for (const unknown of ['elsewhere', 'nowhere', 'nowhere']) { act(() => { result.current.write('/user', [unknown]); }); expect(result.current.router.route).toBe('account'); - expect(result.current.segment).toEqual(['account']); + expect(result.current.segment).toEqual([]); } expect(warn).not.toHaveBeenCalled(); warn.mockRestore(); @@ -157,51 +170,33 @@ describe('useRouterScope', () => { }); expect(result.current.router.route).toBe('account'); - expect(result.current.segment).toEqual(['account']); + expect(result.current.segment).toEqual([]); expect(warn).not.toHaveBeenCalled(); warn.mockRestore(); }); - it('does not redirect to a not-found route that cannot match on its own, and warns', () => { - const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); - const { result } = renderHook( - () => ({ - router: useRouterScope(routes, { routing: 'virtual', path: '/user', notFound: 'statement' }), - segment: useUserSegment(), - }), - { wrapper: wrapperWith({ '/user': ['nowhere'] }) }, - ); - - expect(result.current.router.route).toBe('statement'); - expect(result.current.segment).toEqual(['nowhere']); - expect(warn).toHaveBeenCalledTimes(1); - expect(warn.mock.calls[0][0]).toContain('"statement" under "/user"'); - warn.mockRestore(); - }); - - it('stops redirecting after a burst when two owners of one base redirect each other, and warns', () => { + it('lets two owners of one base agree on the base, without a warning', () => { const warn = vi.spyOn(logger, 'warnOnce').mockImplementation(() => {}); const orgRoutes = { general: 'general', members: 'members' } as const; const { result } = renderHook( () => ({ user: useRouterScope(routes, userScope), - org: useRouterScope(orgRoutes, { routing: 'virtual', path: '/user', notFound: 'general' }), + org: useRouterScope(orgRoutes, { routing: 'virtual', path: '/user', root: 'general' }), segment: useUserSegment(), }), - { wrapper: wrapperWith() }, + { wrapper: wrapperWith({ '/user': ['nowhere'] }) }, ); - expect(['account', 'general']).toContain(result.current.segment[0]); + expect(result.current.segment).toEqual([]); expect(result.current.user.route).toBe('account'); expect(result.current.org.route).toBe('general'); - expect(warn).toHaveBeenCalledTimes(1); - expect(warn.mock.calls[0][0]).toContain('Stopped redirecting under "/user"'); act(() => { result.current.user.go('security'); }); - expect(['security', 'general']).toContain(result.current.segment[0]); + expect(result.current.segment).toEqual([]); + expect(warn).not.toHaveBeenCalled(); warn.mockRestore(); }); @@ -216,7 +211,7 @@ describe('useRouterScope', () => { it('keeps the router while an equal table is passed as a new object', () => { const { result, rerender } = renderHook( - ({ table }: { table: RouteTable }) => useRouterScope(table, { ...userScope, notFound: 'account' }), + ({ table }: { table: RouteTable }) => useRouterScope(table, { ...userScope, root: 'account' }), { initialProps: { table: { ...routes } }, wrapper: wrapperWith({ '/user': ['security'] }) }, ); const { go } = result.current; diff --git a/packages/mosaic/src/routing/use-router.ts b/packages/mosaic/src/routing/use-router.ts index e21e4aa140e..8b2c02c7778 100644 --- a/packages/mosaic/src/routing/use-router.ts +++ b/packages/mosaic/src/routing/use-router.ts @@ -1,11 +1,9 @@ -import { logger } from '@clerk/shared/logger'; import { useEffect, useMemo, useRef, useState } from 'react'; import type { MosaicLocation, SegmentParts } from './location'; import type { Route, Router } from './router'; import { createRouter } from './router'; import type { RouteName, RouteTable } from './routes'; -import { routeParts } from './routes'; import type { RoutingScope } from './routing-scope'; import { createRoutingScope, useInheritedScope } from './routing-scope'; import type { MosaicLocationOptions } from './use-mosaic-location'; @@ -32,34 +30,6 @@ function useStableTable(routes: Routes): Routes { return useStableByKey(routes, Object.entries(routes).flat().join(KEY_SEPARATOR)); } -const REDIRECT_LIMIT = 10; -const REDIRECT_WINDOW_MS = 1000; - -// A write that gives up after a burst. Not-found redirects are automatic, so two scopes with -// different fallbacks on one path would redirect each other forever; a scope that has redirected -// this many times within the window stops and warns instead. No real navigation comes close. -// All attempts to fix this in a more exact fashion turned out to be unnecessarily complex, we only -// need a heuristic to not crash and warn on misconfigurations, not an exact fix. -function withRedirectLimit(location: MosaicLocation, path: string | undefined): MosaicLocation['write'] { - let count = 0; - let since = 0; - return (parts, options) => { - const now = Date.now(); - if (now - since > REDIRECT_WINDOW_MS) { - count = 0; - since = now; - } - count += 1; - if (count > REDIRECT_LIMIT) { - logger.warnOnce( - `[Clerk] Stopped redirecting under "${path ?? '/'}" after ${REDIRECT_LIMIT} redirects within a second. Two components are probably configured with this path; give one of them a different path.`, - ); - return; - } - location.write(parts, options); - }; -} - // The scope owns its base while its effects are live, and returns the entry it should render from. // When the last owner leaves, the location drops the entry, so the next mount starts fresh the way // a URL would after the host navigated away. Effects also stop when React parks the component @@ -138,9 +108,13 @@ function useMatch(router: Router, entry: Segm export interface RouterScopeOptions { routing: MosaicLocationOptions['routing']; path?: string; - // The route a segment no route matches redirects to, with replace. The owner of a segment always - // decides what an unknown segment means, so `route` below is never undefined. - notFound: RouteName; + // The route that lives at the base. It is matched by the empty segment and `go` writes the + // empty segment for it, whatever pattern the table gives it; that pattern is accepted too and + // canonicalised to the base. Any other segment no route matches shows the root and is replaced + // with the base as well. The owner of a segment always decides what an unknown segment means, so + // `route` below is never undefined, and because every owner's fallback is its base, two owners + // of one base cannot redirect each other. + root: RouteName; } export interface RouterScope { @@ -154,38 +128,21 @@ export interface RouterScope { // and it is the only router that redirects. export function useRouterScope( routes: Routes, - { routing, path, notFound }: RouterScopeOptions, + { routing, path, root }: RouterScopeOptions, ): RouterScope { const scope = useMemo(() => createRoutingScope({ routing, path }), [routing, path]); - const { location, table, router, entry } = useRouting(routes, { routing, path }); + const rooted: Routes = Object.assign({}, routes, { [root]: '' }); + const { location, router, entry } = useRouting(rooted, { routing, path }); const held = useOwnedSegment(location, entry, routing === 'virtual'); const { parts, matched } = useMatch(router, held); - const current = useMemo>( - () => matched ?? { name: notFound, params: EMPTY_PARAMS }, - [matched, notFound], - ); - - const fallback = useMemo(() => { - const target = routeParts(table, notFound); - return router.parse(target) ? target : undefined; - }, [table, router, notFound]); + const current = useMemo>(() => matched ?? { name: root, params: EMPTY_PARAMS }, [matched, root]); - const redirect = useMemo(() => withRedirectLimit(location, path), [location, path]); - const redirectTo = matched === undefined ? notFound : undefined; + const unmatched = matched === undefined; useEffect(() => { - if (redirectTo === undefined) { - return; - } - if (fallback === undefined) { - if (Object.keys(table).length > 0) { - logger.warnOnce( - `[Clerk] The notFound route "${redirectTo}" under "${path ?? '/'}" is either missing or needs params, so it cannot be a fallback. Use a route without params.`, - ); - } - return; + if (unmatched) { + location.write(EMPTY_PARTS, { replace: true }); } - redirect(fallback, { replace: true }); - }, [parts, redirectTo, fallback, redirect, table, path]); + }, [parts, unmatched, location]); return useMemo( () => ({ route: current.name, params: current.params, go: router.go, scope }), From 1d28928d85634b707e45fae862e87064477d9572 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Fri, 6 Nov 2026 15:43:46 +0100 Subject: [PATCH 18/26] Introduce ready concept for dynamic routes --- .../organization-profile.tsx | 4 +-- .../profile-pages.controller.test.tsx | 21 ++++++++++++++ .../profile-pages/profile-pages.controller.ts | 11 +++++-- .../features/user-profile/user-profile.tsx | 20 +++++++++---- .../src/routing/__tests__/use-router.test.tsx | 16 ++++++++++ packages/mosaic/src/routing/use-router.ts | 29 ++++++++++++++----- 6 files changed, 84 insertions(+), 17 deletions(-) diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.tsx b/packages/mosaic/src/features/organization-profile/organization-profile.tsx index 402590bfbbb..df8afaa598b 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.tsx +++ b/packages/mosaic/src/features/organization-profile/organization-profile.tsx @@ -85,14 +85,14 @@ export function OrganizationProfile(props: OrganizationProfileProps) { const model = useOrganizationProfileModel(); const pages = model.status === 'ready' ? organizationProfilePanels(model.pages, apiKeysProps) : undefined; const { activePage, onPageChange } = useProfilePagesController( - pages ? organizationProfilePageIds({ pages, customPages, pageOrder }) : [], + pages && organizationProfilePageIds({ pages, customPages, pageOrder }), { routing: 'virtual', path }, ); if (model.status === 'loading') { return fallback ?? null; } - if (!pages) { + if (!pages || activePage === undefined) { return null; } diff --git a/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx b/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx index 58ecc43c999..15f5c8d82b6 100644 --- a/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx +++ b/packages/mosaic/src/features/profile-pages/__tests__/profile-pages.controller.test.tsx @@ -36,6 +36,27 @@ describe('useProfilePagesController', () => { expect(result.current.segment).toBeUndefined(); }); + it('reports no page and leaves the entry alone while the page ids are unknown', () => { + const { result, rerender } = renderHook( + ({ pageIds }: { pageIds: readonly string[] | undefined }) => ({ + pages: useProfilePagesController(pageIds, { routing: 'virtual', path: '/organization' }), + segment: useVirtualSegment('/organization'), + }), + { + initialProps: { pageIds: undefined }, + wrapper: ({ children }) => ( + {children} + ), + }, + ); + expect(result.current.pages.activePage).toBeUndefined(); + expect(result.current.segment).toEqual(['members']); + + rerender({ pageIds: ['general', 'members'] }); + expect(result.current.pages.activePage).toBe('members'); + expect(result.current.segment).toEqual(['members']); + }); + it("canonicalises the root page's own segment to the base", () => { const { result } = renderHook( () => ({ diff --git a/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts b/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts index c190a37eb64..434199a637b 100644 --- a/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts +++ b/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts @@ -4,10 +4,15 @@ import { pageSegment } from './profile-pages.layout'; const NO_PAGE = ''; -export function useProfilePagesController(pageIds: readonly string[], options: MosaicLocationOptions) { - const { route, go } = useRouterScope(Object.fromEntries(pageIds.map(id => [id, pageSegment(id)])), { +// Page ids that are still unknown, because the model they come from is loading, give no active +// page and leave the location alone, so a page written before the profile was ready is still +// there when it is. +export function useProfilePagesController(pageIds: readonly string[] | undefined, options: MosaicLocationOptions) { + const ids = pageIds ?? []; + const { route, go } = useRouterScope(Object.fromEntries(ids.map(id => [id, pageSegment(id)])), { ...options, - root: pageIds[0] ?? NO_PAGE, + root: ids[0] ?? NO_PAGE, + ready: pageIds !== undefined, }); return { activePage: route, onPageChange: go }; diff --git a/packages/mosaic/src/features/user-profile/user-profile.tsx b/packages/mosaic/src/features/user-profile/user-profile.tsx index aa9e9145da8..a05499b31ce 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile.tsx @@ -184,6 +184,7 @@ export function UserProfileSurface({ ) : ( @@ -202,6 +203,10 @@ function AccountUserProfileView({ path, ...viewProps }: AccountViewProps) { path, }); + if (activePage === undefined) { + return null; + } + return ( ) { +}: AccountViewProps & Pick) { const model = useOrganizationProfileModel(); const organization = model.status === 'ready' ? { pages: organizationProfilePanels(model.pages, apiKeysProps), customPages, pageOrder } : undefined; - const { activePage, onPageChange } = useProfilePagesController(userProfilePageIds({ ...viewProps, organization }), { - routing: 'virtual', - path, - }); + const { activePage, onPageChange } = useProfilePagesController( + model.status === 'loading' ? undefined : userProfilePageIds({ ...viewProps, organization }), + { routing: 'virtual', path }, + ); + + if (activePage === undefined) { + return fallback ?? null; + } return ( diff --git a/packages/mosaic/src/routing/__tests__/use-router.test.tsx b/packages/mosaic/src/routing/__tests__/use-router.test.tsx index 17378192f48..ad98d0a4e7a 100644 --- a/packages/mosaic/src/routing/__tests__/use-router.test.tsx +++ b/packages/mosaic/src/routing/__tests__/use-router.test.tsx @@ -200,6 +200,22 @@ describe('useRouterScope', () => { warn.mockRestore(); }); + it('reports no route and writes nothing while not ready, then resolves the entry once it is', () => { + const { result, rerender } = renderHook( + ({ ready }: { ready: boolean }) => ({ + router: useRouterScope(routes, { ...userScope, ready }), + segment: useUserSegment(), + }), + { initialProps: { ready: false }, wrapper: wrapperWith({ '/user': ['security'] }) }, + ); + expect(result.current.router.route).toBeUndefined(); + expect(result.current.segment).toEqual(['security']); + + rerender({ ready: true }); + expect(result.current.router.route).toBe('security'); + expect(result.current.segment).toEqual(['security']); + }); + it('keeps the same result object between unrelated renders', () => { const { result, rerender } = renderHook(() => useRouterScope(routes, userScope), { wrapper: wrapperWith({ '/user': ['security'] }), diff --git a/packages/mosaic/src/routing/use-router.ts b/packages/mosaic/src/routing/use-router.ts index 8b2c02c7778..3c248fd4e98 100644 --- a/packages/mosaic/src/routing/use-router.ts +++ b/packages/mosaic/src/routing/use-router.ts @@ -115,10 +115,14 @@ export interface RouterScopeOptions { // `route` below is never undefined, and because every owner's fallback is its base, two owners // of one base cannot redirect each other. root: RouteName; + // Whether the table is settled. While false the scope reports no route and writes nothing, so an + // entry the table cannot match yet is kept until it can. Pass it when the routes depend on data + // that has not arrived, and handle the undefined route that comes with it. + ready?: boolean; } -export interface RouterScope { - route: RouteName; +export interface RouterScope> { + route: Name; params: Record; go: Router['go']; scope: RoutingScope; @@ -128,16 +132,27 @@ export interface RouterScope { // and it is the only router that redirects. export function useRouterScope( routes: Routes, - { routing, path, root }: RouterScopeOptions, -): RouterScope { + options: RouterScopeOptions & { ready: boolean }, +): RouterScope | undefined>; +export function useRouterScope( + routes: Routes, + options: Omit, 'ready'>, +): RouterScope; +export function useRouterScope( + routes: Routes, + { routing, path, root, ready = true }: RouterScopeOptions, +): RouterScope | undefined> { const scope = useMemo(() => createRoutingScope({ routing, path }), [routing, path]); const rooted: Routes = Object.assign({}, routes, { [root]: '' }); const { location, router, entry } = useRouting(rooted, { routing, path }); const held = useOwnedSegment(location, entry, routing === 'virtual'); const { parts, matched } = useMatch(router, held); - const current = useMemo>(() => matched ?? { name: root, params: EMPTY_PARAMS }, [matched, root]); + const current = useMemo | undefined>( + () => (ready ? (matched ?? { name: root, params: EMPTY_PARAMS }) : undefined), + [ready, matched, root], + ); - const unmatched = matched === undefined; + const unmatched = ready && matched === undefined; useEffect(() => { if (unmatched) { location.write(EMPTY_PARTS, { replace: true }); @@ -145,7 +160,7 @@ export function useRouterScope( }, [parts, unmatched, location]); return useMemo( - () => ({ route: current.name, params: current.params, go: router.go, scope }), + () => ({ route: current?.name, params: current?.params ?? EMPTY_PARAMS, go: router.go, scope }), [router, scope, current], ); } From 8234eeff52843530338c014052e7c0808c7184b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Fri, 6 Nov 2026 15:50:25 +0100 Subject: [PATCH 19/26] Add tests and fix modal bug --- .../__tests__/user-profile.feature.test.tsx | 18 +++++++++ packages/mosaic/src/modals/modal-host.tsx | 7 ++++ packages/mosaic/src/modals/use-modal.test.tsx | 39 ++++++++++++++++++- 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx index b9427f3f91d..1bbf5046172 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx @@ -166,6 +166,24 @@ describe('UserProfile', () => { expect(entry()).toEqual(['api-keys']); }); + it('opens as a modal on the first page when the requested page is unavailable', async () => { + serve({ userAPIKeysEnabled: false }); + await renderWithClerk( + <> + + + , + ); + const user = userEvent.setup(); + + await user.click(screen.getByRole('button', { name: 'Open profile' })); + const dialog = await screen.findByRole('dialog'); + + expect(await within(dialog).findByRole('tab', { name: 'Account' })).toHaveAttribute('aria-selected', 'true'); + expect(within(dialog).queryByRole('tab', { name: 'API Keys' })).not.toBeInTheDocument(); + await waitFor(() => expect(entry()).toEqual([])); + }); + it('opens as a modal on the root page with an empty entry', async () => { serve(); await renderWithClerk( diff --git a/packages/mosaic/src/modals/modal-host.tsx b/packages/mosaic/src/modals/modal-host.tsx index 9838ce724f6..abdb2629cd4 100644 --- a/packages/mosaic/src/modals/modal-host.tsx +++ b/packages/mosaic/src/modals/modal-host.tsx @@ -49,6 +49,13 @@ export function ModalSurfaceHost({ const base = surface.path ?? `/modal/${surface.id}`; const location = useMosaicLocation({ routing: 'virtual', path: base }); const open = location.useSegmentParts() !== undefined; + // The entry may appear without `open` being called, written from outside or restored by a URL, + // so the content loads whenever the modal is open, not only when the handle opened it. + React.useEffect(() => { + if (open) { + surface.loader.send({ type: 'LOAD' }); + } + }, [open, surface]); const modal = React.useMemo>(() => { const preload = () => surface.loader.send({ type: 'LOAD' }); return { diff --git a/packages/mosaic/src/modals/use-modal.test.tsx b/packages/mosaic/src/modals/use-modal.test.tsx index e137a926e7c..400515922fe 100644 --- a/packages/mosaic/src/modals/use-modal.test.tsx +++ b/packages/mosaic/src/modals/use-modal.test.tsx @@ -1,12 +1,12 @@ import { logger } from '@clerk/shared/logger'; -import { act, cleanup, configure, render, renderHook, screen, within } from '@testing-library/react'; +import { act, cleanup, configure, render, renderHook, screen, waitFor, within } from '@testing-library/react'; import React from 'react'; import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest'; import type { MosaicProviderProps } from '../mosaic-provider'; import { MosaicProvider } from '../mosaic-provider'; import { useRouterScope } from '../routing/use-router'; -import { useVirtualSegment } from '../routing/virtual-store'; +import { useVirtualSegment, useVirtualWrite } from '../routing/virtual-store'; import { defineModalSurface } from './define-modal-surface'; import type { ModalContentProps, ModalHandle, ModalSurface, ModalSurfaces } from './modal.types'; import { ModalHost } from './modal-host'; @@ -612,6 +612,41 @@ describe('a modal surface that routes', () => { }); }); + it('opens and closes with outside writes to its base, and isOpen follows', async () => { + const surface = routedSurface(); + const ref: { handle: ModalHandle | undefined; write: ReturnType | undefined } = + { + handle: undefined, + write: undefined, + }; + function Outside({ onWrite }: { onWrite: (write: ReturnType) => void }) { + onWrite(useVirtualWrite()); + return null; + } + renderWithModals( + [surface], + <> + (ref.handle = h)} + /> + (ref.write = w)} /> + + , + ); + expect(ref.handle?.isOpen).toBe(false); + + act(() => ref.write?.('/modal/stub', ['security'])); + await flush(); + expect(ref.handle?.isOpen).toBe(true); + expect(screen.getByRole('dialog')).toBeInTheDocument(); + + act(() => ref.write?.('/modal/stub', undefined)); + await flush(); + expect(ref.handle?.isOpen).toBe(false); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + }); + it('hands its base to the content', async () => { const surface = defineModalSurface({ id: 'based', From 7b065ea2353491cadd346980a072604bc2461deb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Wed, 11 Nov 2026 12:42:45 +0100 Subject: [PATCH 20/26] Implement notFound for routing Makes organization routes in combined mode fallback "general" within its own section again. This was implemented on main but removed during a rebase and this is the routing version of that. --- .../organization-profile.tsx | 4 +-- .../profile-pages/profile-pages.controller.ts | 7 +++- .../__tests__/user-profile.feature.test.tsx | 18 ++++++++-- .../features/user-profile/user-profile.tsx | 13 +++++--- .../src/routing/__tests__/use-router.test.tsx | 23 +++++++++++++ packages/mosaic/src/routing/use-router.ts | 33 +++++++++++++------ 6 files changed, 79 insertions(+), 19 deletions(-) diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.tsx b/packages/mosaic/src/features/organization-profile/organization-profile.tsx index df8afaa598b..870cccada83 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.tsx +++ b/packages/mosaic/src/features/organization-profile/organization-profile.tsx @@ -19,8 +19,8 @@ import { OrganizationProfileMembersPanel } from './organization-profile-members- /** Everything `` takes: your own pages and their order, the page it opens on, and where leaving the organization lands. */ export interface OrganizationProfileProps extends - Pick, - Pick { + Pick, + Pick { /** * The base the profile's pages are nested under. Two profiles on one page need different bases. * diff --git a/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts b/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts index 434199a637b..10cd0284316 100644 --- a/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts +++ b/packages/mosaic/src/features/profile-pages/profile-pages.controller.ts @@ -1,13 +1,18 @@ +import type { SegmentParts } from '../../routing/location'; import type { MosaicLocationOptions } from '../../routing/use-mosaic-location'; import { useRouterScope } from '../../routing/use-router'; import { pageSegment } from './profile-pages.layout'; +export interface ProfilePagesOptions extends MosaicLocationOptions { + notFound?: (parts: SegmentParts) => string | undefined; +} + const NO_PAGE = ''; // Page ids that are still unknown, because the model they come from is loading, give no active // page and leave the location alone, so a page written before the profile was ready is still // there when it is. -export function useProfilePagesController(pageIds: readonly string[] | undefined, options: MosaicLocationOptions) { +export function useProfilePagesController(pageIds: readonly string[] | undefined, options: ProfilePagesOptions) { const ids = pageIds ?? []; const { route, go } = useRouterScope(Object.fromEntries(ids.map(id => [id, pageSegment(id)])), { ...options, diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx index 1bbf5046172..29497e0a12b 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile.feature.test.tsx @@ -370,6 +370,20 @@ describe('UserProfile with an active organization', () => { expect(screen.getByRole('heading', { level: 3, name: 'Members' })).toBeVisible(); }); + it('opens as a modal on the first organization page when the requested organization page is unavailable', async () => { + serveWithOrganization({ permissions: [] }); + await renderWithClerk(); + const user = userEvent.setup(); + + await user.click(screen.getByRole('button', { name: 'Open profile' })); + const dialog = await screen.findByRole('dialog'); + + expect(await within(dialog).findByRole('tab', { name: 'General', description: 'Organization' })).toHaveAttribute( + 'aria-selected', + 'true', + ); + }); + it('opens the organization page that shares an account page name', async () => { serveWithOrganization(); await renderWithClerk( @@ -415,7 +429,7 @@ describe('UserProfile with an active organization', () => { expect(tabsDescribedBy('Organization')).toEqual(['Audit log', 'General', 'Members']); }); - it('keeps the shown page after the open organization page leaves the navigation', async () => { + it('stays in the organization group after the open organization page leaves the navigation', async () => { const globex = fapiOrganization({ id: 'org_2', name: 'Globex' }); const memberships = [ fapiMembership(acme, { permissions: ['org:sys_memberships:read'] }), @@ -435,7 +449,7 @@ describe('UserProfile with an active organization', () => { await act(() => clerk.setActive({ organization: acme.id })); await waitFor(() => expect(tabsDescribedBy('Organization')).toEqual(['General', 'Members'])); - expect(screen.getByRole('tab', { name: 'Account', description: 'Account' })).toHaveAttribute( + expect(screen.getByRole('tab', { name: 'General', description: 'Organization' })).toHaveAttribute( 'aria-selected', 'true', ); diff --git a/packages/mosaic/src/features/user-profile/user-profile.tsx b/packages/mosaic/src/features/user-profile/user-profile.tsx index a05499b31ce..abf0e1960ee 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile.tsx @@ -227,10 +227,15 @@ function CombinedUserProfileView({ model.status === 'ready' ? { pages: organizationProfilePanels(model.pages, apiKeysProps), customPages, pageOrder } : undefined; - const { activePage, onPageChange } = useProfilePagesController( - model.status === 'loading' ? undefined : userProfilePageIds({ ...viewProps, organization }), - { routing: 'virtual', path }, - ); + const pageIds = model.status === 'loading' ? undefined : userProfilePageIds({ ...viewProps, organization }); + const { activePage, onPageChange } = useProfilePagesController(pageIds, { + routing: 'virtual', + path, + notFound: parts => + `${parts[0]}/` === ORGANIZATION_PAGE_PREFIX + ? pageIds?.find(id => id.startsWith(ORGANIZATION_PAGE_PREFIX)) + : undefined, + }); if (activePage === undefined) { return fallback ?? null; diff --git a/packages/mosaic/src/routing/__tests__/use-router.test.tsx b/packages/mosaic/src/routing/__tests__/use-router.test.tsx index ad98d0a4e7a..66019763e27 100644 --- a/packages/mosaic/src/routing/__tests__/use-router.test.tsx +++ b/packages/mosaic/src/routing/__tests__/use-router.test.tsx @@ -216,6 +216,29 @@ describe('useRouterScope', () => { expect(result.current.segment).toEqual(['security']); }); + it('asks notFound where an unknown segment goes', () => { + const notFound = vi.fn((parts: readonly string[]) => (parts[0] === 'billing' ? ('statement' as const) : undefined)); + const statementRoutes = { account: 'account', security: 'security', statement: 'billing/statement' } as const; + const { result } = renderHook( + () => ({ + router: useRouterScope(statementRoutes, { routing: 'virtual', path: '/user', root: 'account', notFound }), + segment: useUserSegment(), + write: useVirtualWrite(), + }), + { wrapper: wrapperWith({ '/user': ['billing', 'nowhere'] }) }, + ); + expect(result.current.router.route).toBe('statement'); + expect(result.current.segment).toEqual(['billing', 'statement']); + expect(notFound).toHaveBeenLastCalledWith(['billing', 'nowhere']); + + act(() => { + result.current.write('/user', ['nowhere']); + }); + expect(notFound).toHaveBeenLastCalledWith(['nowhere']); + expect(result.current.router.route).toBe('account'); + expect(result.current.segment).toEqual([]); + }); + it('keeps the same result object between unrelated renders', () => { const { result, rerender } = renderHook(() => useRouterScope(routes, userScope), { wrapper: wrapperWith({ '/user': ['security'] }), diff --git a/packages/mosaic/src/routing/use-router.ts b/packages/mosaic/src/routing/use-router.ts index 3c248fd4e98..477264cbba2 100644 --- a/packages/mosaic/src/routing/use-router.ts +++ b/packages/mosaic/src/routing/use-router.ts @@ -4,6 +4,7 @@ import type { MosaicLocation, SegmentParts } from './location'; import type { Route, Router } from './router'; import { createRouter } from './router'; import type { RouteName, RouteTable } from './routes'; +import { routeParts } from './routes'; import type { RoutingScope } from './routing-scope'; import { createRoutingScope, useInheritedScope } from './routing-scope'; import type { MosaicLocationOptions } from './use-mosaic-location'; @@ -119,6 +120,10 @@ export interface RouterScopeOptions { // entry the table cannot match yet is kept until it can. Pass it when the routes depend on data // that has not arrived, and handle the undefined route that comes with it. ready?: boolean; + // Where a segment nothing matches goes instead of the root, asked with the segment; a profile + // sends an unknown page in the organization group to that group's first page. Returning nothing, + // or a name that is not a route, means the root. + notFound?: (parts: SegmentParts) => RouteName | undefined; } export interface RouterScope> { @@ -140,24 +145,32 @@ export function useRouterScope( ): RouterScope; export function useRouterScope( routes: Routes, - { routing, path, root, ready = true }: RouterScopeOptions, + { routing, path, root, ready = true, notFound }: RouterScopeOptions, ): RouterScope | undefined> { const scope = useMemo(() => createRoutingScope({ routing, path }), [routing, path]); const rooted: Routes = Object.assign({}, routes, { [root]: '' }); - const { location, router, entry } = useRouting(rooted, { routing, path }); + const { location, table, router, entry } = useRouting(rooted, { routing, path }); const held = useOwnedSegment(location, entry, routing === 'virtual'); const { parts, matched } = useMatch(router, held); - const current = useMemo | undefined>( - () => (ready ? (matched ?? { name: root, params: EMPTY_PARAMS }) : undefined), - [ready, matched, root], - ); + const fallback = useMemo | undefined>(() => { + if (!ready || matched !== undefined) { + return undefined; + } + const name = notFound?.(parts); + return name !== undefined && name in table ? name : root; + }, [ready, matched, notFound, parts, table, root]); + const current = useMemo | undefined>(() => { + if (!ready) { + return undefined; + } + return matched ?? { name: fallback ?? root, params: EMPTY_PARAMS }; + }, [ready, matched, fallback, root]); - const unmatched = ready && matched === undefined; useEffect(() => { - if (unmatched) { - location.write(EMPTY_PARTS, { replace: true }); + if (fallback !== undefined) { + location.write(routeParts(table, fallback), { replace: true }); } - }, [parts, unmatched, location]); + }, [parts, fallback, location, table]); return useMemo( () => ({ route: current?.name, params: current?.params ?? EMPTY_PARAMS, go: router.go, scope }), From 5b9942153ddba47a201842b3439c0ecd675f4a38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Wed, 11 Nov 2026 12:46:42 +0100 Subject: [PATCH 21/26] Fix tests after rebase --- .../eslint/__tests__/boundaries.test.mjs | 22 +++++++++++++++++++ .../switcher.profile.feature.test.tsx | 3 ++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/mosaic/eslint/__tests__/boundaries.test.mjs b/packages/mosaic/eslint/__tests__/boundaries.test.mjs index 29b1948b0ab..89a96633a42 100644 --- a/packages/mosaic/eslint/__tests__/boundaries.test.mjs +++ b/packages/mosaic/eslint/__tests__/boundaries.test.mjs @@ -201,6 +201,16 @@ ruleTester.run('dependencies', boundaries.rules.dependencies, { filename: 'src/modals/sample.ts', code: "import { setup } from '../machine/setup';", }, + { + name: "src/modals/sample.ts may import down or across: import { useMosaicLocation } from '../routing/use-mosaic-location';", + filename: 'src/modals/sample.ts', + code: "import { useMosaicLocation } from '../routing/use-mosaic-location';", + }, + { + name: "src/features/reverification/sample.controller.ts may import down or across: import { useRouterScope } from '../../routing/use-router';", + filename: 'src/features/reverification/sample.controller.ts', + code: "import { useRouterScope } from '../../routing/use-router';", + }, { name: "src/blocks/sample.tsx may import down or across: import { Reverification } from '../features/reverification';", filename: 'src/blocks/sample.tsx', @@ -425,5 +435,17 @@ ruleTester.run('dependencies', boundaries.rules.dependencies, { code: "import { Button } from '../components/button';", errors: [/down/], }, + { + name: "src/routing/sample.ts cannot import up: import { ModalHost } from '../modals/modal-host';", + filename: 'src/routing/sample.ts', + code: "import { ModalHost } from '../modals/modal-host';", + errors: [/down/], + }, + { + name: "src/components/sample.tsx cannot import up: import { useRouter } from '../routing/use-router';", + filename: 'src/components/sample.tsx', + code: "import { useRouter } from '../routing/use-router';", + errors: [/down/], + }, ].map(testCase => ({ ...testCase, options, settings: config.settings })), }); diff --git a/packages/mosaic/src/features/switcher/__tests__/switcher.profile.feature.test.tsx b/packages/mosaic/src/features/switcher/__tests__/switcher.profile.feature.test.tsx index 37a9cdb2912..a0cefa99b45 100644 --- a/packages/mosaic/src/features/switcher/__tests__/switcher.profile.feature.test.tsx +++ b/packages/mosaic/src/features/switcher/__tests__/switcher.profile.feature.test.tsx @@ -334,9 +334,10 @@ describe('a mounted UserProfile under a provider profile', () => { }); it('lands where the provider says after leaving the organization', async () => { - const { navigate } = await renderWithProfile(, { + const { navigate } = await renderWithProfile(, { organizationProfileProps: { afterLeaveOrganizationUrl: '/left' }, }); + await userEvent.setup().click(await screen.findByRole('tab', { name: 'General', description: 'Organization' })); await leaveOrganization(); await waitFor(() => expect(navigate).toHaveBeenCalledWith('/left')); From 220844b23eed2da07859e18e4dc0920765a2c720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Wed, 11 Nov 2026 12:56:45 +0100 Subject: [PATCH 22/26] Add path prop to top level profile props --- .../organization-profile.tsx | 2 +- .../profile-pages/use-mosaic-profile.ts | 4 ++-- .../switcher.profile.feature.test.tsx | 18 ++++++++++++++++++ .../src/features/user-profile/user-profile.tsx | 2 ++ 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/packages/mosaic/src/features/organization-profile/organization-profile.tsx b/packages/mosaic/src/features/organization-profile/organization-profile.tsx index 870cccada83..04af3447fe3 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile.tsx +++ b/packages/mosaic/src/features/organization-profile/organization-profile.tsx @@ -77,7 +77,7 @@ export function OrganizationProfile(props: OrganizationProfileProps) { const { customPages = organizationProfileProps?.customPages, pageOrder = organizationProfileProps?.pageOrder, - path = '/organization', + path = organizationProfileProps?.path ?? '/organization', afterLeaveOrganizationUrl = organizationProfileProps?.afterLeaveOrganizationUrl, fallback, apiKeysProps = organizationProfileProps?.apiKeysProps, diff --git a/packages/mosaic/src/features/profile-pages/use-mosaic-profile.ts b/packages/mosaic/src/features/profile-pages/use-mosaic-profile.ts index 820d90ff8ce..15b77d41d12 100644 --- a/packages/mosaic/src/features/profile-pages/use-mosaic-profile.ts +++ b/packages/mosaic/src/features/profile-pages/use-mosaic-profile.ts @@ -50,13 +50,13 @@ export interface ProfilePages { /** Options for the underlying `` component. */ export interface ProfileUserProps - extends ProfilePages, Pick {} + extends ProfilePages, Pick {} /** Options for the underlying `` component. */ export interface ProfileOrganizationProps extends ProfilePages, - Pick {} + Pick {} const MosaicProfileContext = createContext({}); diff --git a/packages/mosaic/src/features/switcher/__tests__/switcher.profile.feature.test.tsx b/packages/mosaic/src/features/switcher/__tests__/switcher.profile.feature.test.tsx index a0cefa99b45..e413d5f6f12 100644 --- a/packages/mosaic/src/features/switcher/__tests__/switcher.profile.feature.test.tsx +++ b/packages/mosaic/src/features/switcher/__tests__/switcher.profile.feature.test.tsx @@ -17,6 +17,7 @@ import { } from '../../../__tests__/feature/fapi'; import { openMenu } from '../../../__tests__/feature/menu'; import { renderWithClerk } from '../../../__tests__/feature/render'; +import { useVirtualEntries } from '../../../routing/virtual-store'; import { OrganizationProfile } from '../../organization-profile/organization-profile'; import { OrganizationSwitcher } from '../../organization-switcher/organization-switcher'; import type { MosaicProfile } from '../../profile-pages/use-mosaic-profile'; @@ -324,7 +325,24 @@ describe('a mounted OrganizationProfile under a provider profile', () => { }); }); +function Entry({ base }: { base: string }) { + return {JSON.stringify(useVirtualEntries().get(base) ?? null)}; +} + describe('a mounted UserProfile under a provider profile', () => { + it("routes its pages under the provider's path", async () => { + await renderWithProfile( + <> + + + , + { userProfileProps: { path: '/account' } }, + ); + + await userEvent.setup().click(await screen.findByRole('tab', { name: 'Security' })); + expect(JSON.parse(screen.getByTestId('entry').textContent ?? 'null')).toEqual(['security']); + }); + it('fills in what its own organizationProfileProps leave out from the provider', async () => { await renderWithProfile(, { organizationProfileProps: { customPages: [{ ...terms, label: 'Audit', path: 'audit' }], pageOrder: ['general'] }, diff --git a/packages/mosaic/src/features/user-profile/user-profile.tsx b/packages/mosaic/src/features/user-profile/user-profile.tsx index abf0e1960ee..a67e7ea65da 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile.tsx @@ -113,6 +113,7 @@ export function UserProfile(props: UserProfileProps) { pageOrder = userProfileProps?.pageOrder, additionalOAuthScopes = userProfileProps?.additionalOAuthScopes, apiKeysProps = userProfileProps?.apiKeysProps, + path = userProfileProps?.path, organizationProfileProps: { customPages: organizationCustomPages = providerOrganization?.customPages, pageOrder: organizationPageOrder = providerOrganization?.pageOrder, @@ -130,6 +131,7 @@ export function UserProfile(props: UserProfileProps) { pageOrder={pageOrder} additionalOAuthScopes={additionalOAuthScopes} apiKeysProps={apiKeysProps} + path={path} organizationProfileProps={{ customPages: organizationCustomPages, pageOrder: organizationPageOrder, From 5bdedbbfb439fbb636a49fa7d1671572924cce0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Wed, 11 Nov 2026 14:06:38 +0100 Subject: [PATCH 23/26] Reject nameless params --- packages/mosaic/src/routing/__tests__/routes.test.ts | 5 +++++ packages/mosaic/src/routing/routes.ts | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/mosaic/src/routing/__tests__/routes.test.ts b/packages/mosaic/src/routing/__tests__/routes.test.ts index 3001716808a..7688b5ae828 100644 --- a/packages/mosaic/src/routing/__tests__/routes.test.ts +++ b/packages/mosaic/src/routing/__tests__/routes.test.ts @@ -71,6 +71,11 @@ describe('compilePattern', () => { expect(() => compilePattern('api%20keys')).toThrow('written decoded'); }); + it('rejects a param without a name', () => { + expect(() => compilePattern('billing/:')).toThrow('without a name'); + expect(() => compilePattern('billing/:?')).toThrow('without a name'); + }); + it('rejects a required part after an optional one', () => { expect(() => compilePattern('a/:b?/c')).toThrow('optional parts come last'); expect(() => compilePattern('a/:b?/:c')).toThrow('optional parts come last'); diff --git a/packages/mosaic/src/routing/routes.ts b/packages/mosaic/src/routing/routes.ts index f989cb61ca8..682b30680c3 100644 --- a/packages/mosaic/src/routing/routes.ts +++ b/packages/mosaic/src/routing/routes.ts @@ -71,7 +71,11 @@ export function compilePattern(pattern: string): PatternPart[] { return { param: false, value: part }; } const optional = part.endsWith('?'); - return { param: true, name: part.slice(1, optional ? -1 : undefined), optional }; + const name = part.slice(1, optional ? -1 : undefined); + if (name === '') { + throw new Error(`[clerk] Route pattern "${pattern}" has a param without a name.`); + } + return { param: true, name, optional }; }); const firstOptional = parts.findIndex(part => part.param && part.optional); if (firstOptional !== -1 && parts.slice(firstOptional).some(part => !part.param || !part.optional)) { From 1c4dbddcb08a9672f4e3f993f8bfe90979606c41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Wed, 11 Nov 2026 15:31:36 +0100 Subject: [PATCH 24/26] Fix modal abstraction stale state reuse --- packages/mosaic/src/modals/modal-host.tsx | 49 ++++++++++++++++--- packages/mosaic/src/modals/use-modal.test.tsx | 39 +++++++++++++++ 2 files changed, 81 insertions(+), 7 deletions(-) diff --git a/packages/mosaic/src/modals/modal-host.tsx b/packages/mosaic/src/modals/modal-host.tsx index abdb2629cd4..6496c50aea6 100644 --- a/packages/mosaic/src/modals/modal-host.tsx +++ b/packages/mosaic/src/modals/modal-host.tsx @@ -40,6 +40,8 @@ export function ModalSurfaceHost({ options: undefined, container: undefined, openCount: 0, + openRequested: false, + wasOpen: false, }); // The modal open state is derived from the routing state, so there is no state to sync and @@ -48,27 +50,55 @@ export function ModalSurfaceHost({ // the Back button just works etc. The public modal open/close are just routing writes. const base = surface.path ?? `/modal/${surface.id}`; const location = useMosaicLocation({ routing: 'virtual', path: base }); - const open = location.useSegmentParts() !== undefined; + const isOpen = location.useSegmentParts() !== undefined; + // wasOpen guarantees this only runs once as we are opening the modal + if (isOpen && !state.wasOpen) { + setState(previous => + // openRequested tracks if it was opened through an explicit open() call, + // if it was we want to use the options/container from there, otherwise the + // modal is rendering because the routing state says so, and we reset whatever + // options/container state was lying around since last time the modal was open. + previous.openRequested + ? { ...previous, openRequested: false, wasOpen: true } + : { + options: undefined, + container: undefined, + openCount: previous.openCount + 1, + openRequested: false, + wasOpen: true, + }, + ); + } else if (!isOpen && state.wasOpen) { + // Runs once as we are closing, only records wasOpen: false, we can't clear the + // full state here as we need it around for the exit transition + setState(previous => ({ ...previous, wasOpen: false })); + } // The entry may appear without `open` being called, written from outside or restored by a URL, // so the content loads whenever the modal is open, not only when the handle opened it. React.useEffect(() => { - if (open) { + if (isOpen) { surface.loader.send({ type: 'LOAD' }); } - }, [open, surface]); + }, [isOpen, surface]); const modal = React.useMemo>(() => { const preload = () => surface.loader.send({ type: 'LOAD' }); return { open: (options, container) => { preload(); - setState(previous => ({ options, container, openCount: previous.openCount + 1 })); + setState(previous => ({ + ...previous, + options, + container, + openCount: previous.openCount + 1, + openRequested: !isOpen, + })); location.write(surface.initialParts?.(options) ?? []); }, close: () => location.clear(), preload, - isOpen: open, + isOpen, }; - }, [surface, location, open]); + }, [surface, location, isOpen]); return ( @@ -76,7 +106,7 @@ export function ModalSurfaceHost({ @@ -143,4 +173,9 @@ interface ModalState { options: Options | undefined; container: PortalContainer | undefined; openCount: number; + // Set by `open()` until the opening it asked for happens. An opening without it came from outside. + openRequested: boolean; + // `isOpen` as of the previous render, kept only to tell the opening and closing edges apart. The + // entry alone decides whether the modal renders. + wasOpen: boolean; } diff --git a/packages/mosaic/src/modals/use-modal.test.tsx b/packages/mosaic/src/modals/use-modal.test.tsx index 400515922fe..0e636061367 100644 --- a/packages/mosaic/src/modals/use-modal.test.tsx +++ b/packages/mosaic/src/modals/use-modal.test.tsx @@ -647,6 +647,45 @@ describe('a modal surface that routes', () => { await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); }); + it('does not reuse the options or container of an earlier open for an opening written from outside', async () => { + const container = document.body.appendChild(document.createElement('div')); + onTestFinished(() => container.remove()); + const surface = routedSurface(); + const ref: { handle: ModalHandle | undefined; write: ReturnType | undefined } = + { + handle: undefined, + write: undefined, + }; + function Outside({ onWrite }: { onWrite: (write: ReturnType) => void }) { + onWrite(useVirtualWrite()); + return null; + } + renderWithModals( + [surface], + <> + (ref.handle = h)} + /> + (ref.write = w)} /> + , + ); + + act(() => ref.handle?.open({ label: 'first' }, container)); + await flush(); + expect(readOptions()).toEqual({ label: 'first' }); + expect(container).toContainElement(screen.getByLabelText('Stub')); + + act(() => ref.handle?.close()); + await flush(); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + + act(() => ref.write?.('/modal/stub', [])); + await flush(); + expect(readOptions()).toBeNull(); + expect(container).not.toContainElement(screen.getByLabelText('Stub')); + }); + it('hands its base to the content', async () => { const surface = defineModalSurface({ id: 'based', From 643d273533278d9ef77af19e95070d82d0661bda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Wed, 11 Nov 2026 15:32:07 +0100 Subject: [PATCH 25/26] Make user profile modal respect options passed through provider --- .../switcher.profile.feature.test.tsx | 18 +++++++++++++++++- .../user-profile.modal-content.tsx | 9 +++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/packages/mosaic/src/features/switcher/__tests__/switcher.profile.feature.test.tsx b/packages/mosaic/src/features/switcher/__tests__/switcher.profile.feature.test.tsx index e413d5f6f12..3dddb2858ce 100644 --- a/packages/mosaic/src/features/switcher/__tests__/switcher.profile.feature.test.tsx +++ b/packages/mosaic/src/features/switcher/__tests__/switcher.profile.feature.test.tsx @@ -17,7 +17,7 @@ import { } from '../../../__tests__/feature/fapi'; import { openMenu } from '../../../__tests__/feature/menu'; import { renderWithClerk } from '../../../__tests__/feature/render'; -import { useVirtualEntries } from '../../../routing/virtual-store'; +import { useVirtualEntries, useVirtualWrite } from '../../../routing/virtual-store'; import { OrganizationProfile } from '../../organization-profile/organization-profile'; import { OrganizationSwitcher } from '../../organization-switcher/organization-switcher'; import type { MosaicProfile } from '../../profile-pages/use-mosaic-profile'; @@ -329,6 +329,22 @@ function Entry({ base }: { base: string }) { return {JSON.stringify(useVirtualEntries().get(base) ?? null)}; } +function OpenByRouting({ base }: { base: string }) { + const write = useVirtualWrite(); + return ; +} + +describe('a user profile modal opened by the routing state', () => { + it("renders with the provider's options", async () => { + await renderWithProfile(, { userProfileProps: { customPages: [terms] } }); + + await userEvent.setup().click(screen.getByRole('button', { name: 'Open by routing' })); + const dialog = await screen.findByRole('dialog'); + + expect(await within(dialog).findByRole('tab', { name: 'Terms' })).toBeInTheDocument(); + }); +}); + describe('a mounted UserProfile under a provider profile', () => { it("routes its pages under the provider's path", async () => { await renderWithProfile( diff --git a/packages/mosaic/src/features/user-profile/user-profile.modal-content.tsx b/packages/mosaic/src/features/user-profile/user-profile.modal-content.tsx index fab98950e9d..06daea7bfcb 100644 --- a/packages/mosaic/src/features/user-profile/user-profile.modal-content.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile.modal-content.tsx @@ -1,8 +1,9 @@ import { useUser } from '@clerk/shared/react'; import React from 'react'; +import { useMosaicProfile, userProfileModalOptions } from '../profile-pages/use-mosaic-profile'; import { UserProfileSurface } from './user-profile'; -import type { UserProfileModalContentProps } from './user-profile.modal.types'; +import type { UserProfileModalContentProps, UserProfileModalOptions } from './user-profile.modal.types'; export default function UserProfileModalContent({ options, @@ -10,6 +11,7 @@ export default function UserProfileModalContent({ path, }: UserProfileModalContentProps): React.ReactElement { const { isLoaded, user } = useUser(); + const profile = useMosaicProfile(); const signedOut = isLoaded && !user; React.useEffect(() => { @@ -18,7 +20,10 @@ export default function UserProfileModalContent({ } }, [signedOut, close]); - const { page: _page, ...props } = options ?? {}; + // An opening the routing state caused on its own, a deep link or a return, carries no options; + // it gets the provider's, the same ones every handle-driven open spreads in. + const provided: UserProfileModalOptions = userProfileModalOptions(profile); + const { page: _page, ...props } = options ?? provided; return ( Date: Wed, 11 Nov 2026 16:08:43 +0100 Subject: [PATCH 26/26] Fix a few edge cases --- packages/mosaic/src/modals/modal-host.tsx | 5 ++- packages/mosaic/src/modals/use-modal.test.tsx | 37 +++++++++++++++++++ .../src/routing/__tests__/routes.test.ts | 14 +++++++ packages/mosaic/src/routing/routes.ts | 16 ++++++-- packages/mosaic/src/routing/use-router.ts | 4 +- 5 files changed, 69 insertions(+), 7 deletions(-) diff --git a/packages/mosaic/src/modals/modal-host.tsx b/packages/mosaic/src/modals/modal-host.tsx index 6496c50aea6..910f6141887 100644 --- a/packages/mosaic/src/modals/modal-host.tsx +++ b/packages/mosaic/src/modals/modal-host.tsx @@ -94,7 +94,10 @@ export function ModalSurfaceHost({ })); location.write(surface.initialParts?.(options) ?? []); }, - close: () => location.clear(), + close: () => { + location.clear(); + setState(previous => (previous.openRequested ? { ...previous, openRequested: false } : previous)); + }, preload, isOpen, }; diff --git a/packages/mosaic/src/modals/use-modal.test.tsx b/packages/mosaic/src/modals/use-modal.test.tsx index 0e636061367..c9966536346 100644 --- a/packages/mosaic/src/modals/use-modal.test.tsx +++ b/packages/mosaic/src/modals/use-modal.test.tsx @@ -686,6 +686,43 @@ describe('a modal surface that routes', () => { expect(container).not.toContainElement(screen.getByLabelText('Stub')); }); + it('forgets a cancelled open, so an opening written from outside starts fresh', async () => { + const container = document.body.appendChild(document.createElement('div')); + onTestFinished(() => container.remove()); + const surface = routedSurface(); + const ref: { handle: ModalHandle | undefined; write: ReturnType | undefined } = + { + handle: undefined, + write: undefined, + }; + function Outside({ onWrite }: { onWrite: (write: ReturnType) => void }) { + onWrite(useVirtualWrite()); + return null; + } + renderWithModals( + [surface], + <> + (ref.handle = h)} + /> + (ref.write = w)} /> + , + ); + + act(() => { + ref.handle?.open({ label: 'cancelled' }, container); + ref.handle?.close(); + }); + await flush(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + + act(() => ref.write?.('/modal/stub', [])); + await flush(); + expect(readOptions()).toBeNull(); + expect(container).not.toContainElement(screen.getByLabelText('Stub')); + }); + it('hands its base to the content', async () => { const surface = defineModalSurface({ id: 'based', diff --git a/packages/mosaic/src/routing/__tests__/routes.test.ts b/packages/mosaic/src/routing/__tests__/routes.test.ts index 7688b5ae828..0a1dfd86f38 100644 --- a/packages/mosaic/src/routing/__tests__/routes.test.ts +++ b/packages/mosaic/src/routing/__tests__/routes.test.ts @@ -116,6 +116,15 @@ describe('buildParts', () => { }); }); +describe('buildParts with a gap', () => { + it('rejects a param given after an omitted optional one', () => { + const [route] = compileRoutes({ r: 'a/:b?/:c?' }); + expect(buildParts(route.parts, { b: 'x', c: 'y' })).toEqual(['a', 'x', 'y']); + expect(buildParts(route.parts, { b: 'x' })).toEqual(['a', 'x']); + expect(() => buildParts(route.parts, { c: 'y' })).toThrow('without "b" before it'); + }); +}); + describe('routeParts', () => { const table = { account: 'account', apiKeys: 'api-keys', statement: 'billing/statement/:statementId' }; @@ -124,6 +133,11 @@ describe('routeParts', () => { expect(routeParts(table, 'statement', { statementId: 'st_1' })).toEqual(['billing', 'statement', 'st_1']); }); + it('builds no parts for an inherited name', () => { + expect(routeParts({ account: 'account' }, 'toString')).toEqual([]); + expect(routeParts({ account: 'account' }, 'constructor')).toEqual([]); + }); + it('builds no parts for an unknown name', () => { expect(routeParts(table, 'nowhere')).toEqual([]); }); diff --git a/packages/mosaic/src/routing/routes.ts b/packages/mosaic/src/routing/routes.ts index 682b30680c3..22141788346 100644 --- a/packages/mosaic/src/routing/routes.ts +++ b/packages/mosaic/src/routing/routes.ts @@ -97,7 +97,7 @@ function minLengthOf(parts: readonly PatternPart[]): number { return minLength; } -function isRouteName(routes: Routes, name: string): name is RouteName { +export function isRouteName(routes: Routes, name: string): name is RouteName { return Object.prototype.hasOwnProperty.call(routes, name); } @@ -156,22 +156,30 @@ export function routeParts( name: string, params: Record = {}, ): string[] { - const pattern = routes[name]; + const pattern = isRouteName(routes, name) ? routes[name] : undefined; return pattern === undefined ? [] : buildParts(compilePattern(pattern), params); } // compiled 'billing/plans/:planId?' + { planId: 'pro' } -> ['billing', 'plans', 'pro'] + {} -> ['billing', 'plans'] +// A param after an omitted optional one has no place to go: it would take the omitted one's +// position and read back under its name, so it is a caller error rather than a shorter segment. export function buildParts(parts: readonly PatternPart[], params: Record): string[] { const built: string[] = []; + let omitted: string | undefined; for (const part of parts) { if (!part.param) { built.push(part.value); continue; } const value = params[part.name]; - if (value !== undefined) { - built.push(value); + if (value === undefined) { + omitted ??= part.name; + continue; + } + if (omitted !== undefined) { + throw new Error(`[clerk] Route param "${part.name}" was given without "${omitted}" before it.`); } + built.push(value); } return built; } diff --git a/packages/mosaic/src/routing/use-router.ts b/packages/mosaic/src/routing/use-router.ts index 477264cbba2..4c5d9fd7e47 100644 --- a/packages/mosaic/src/routing/use-router.ts +++ b/packages/mosaic/src/routing/use-router.ts @@ -4,7 +4,7 @@ import type { MosaicLocation, SegmentParts } from './location'; import type { Route, Router } from './router'; import { createRouter } from './router'; import type { RouteName, RouteTable } from './routes'; -import { routeParts } from './routes'; +import { isRouteName, routeParts } from './routes'; import type { RoutingScope } from './routing-scope'; import { createRoutingScope, useInheritedScope } from './routing-scope'; import type { MosaicLocationOptions } from './use-mosaic-location'; @@ -157,7 +157,7 @@ export function useRouterScope( return undefined; } const name = notFound?.(parts); - return name !== undefined && name in table ? name : root; + return name !== undefined && isRouteName(table, name) ? name : root; }, [ready, matched, notFound, parts, table, root]); const current = useMemo | undefined>(() => { if (!ready) {