Skip to content

Commit 03d324b

Browse files
committed
refactor(ui): shared profile UI infra (style cache, appearance overrides, StrictMode fix) (#9131)
1 parent e60c4ff commit 03d324b

13 files changed

Lines changed: 399 additions & 49 deletions

File tree

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
import { StrictMode } from 'react';
2+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
3+
4+
// The global vitest setup mocks @formkit/auto-animate to a no-op stub. Restore
5+
// the real module for this file (and for Animated.tsx's transitive import) so
6+
// the production useSafeAutoAnimate wiring actually runs.
7+
vi.mock('@formkit/auto-animate', async importOriginal => await importOriginal());
8+
vi.mock('@formkit/auto-animate/react', async importOriginal => await importOriginal());
9+
10+
import { bindCreateFixtures } from '@/test/create-fixtures';
11+
import { render, screen } from '@/test/utils';
12+
13+
import { Animated } from '../../elements/Animated';
14+
15+
const { createFixtures } = bindCreateFixtures('UserProfile');
16+
17+
/**
18+
* Exercises the production <Animated> wrapper (which uses useSafeAutoAnimate)
19+
* under React StrictMode's mount → cleanup → remount cycle. StrictMode
20+
* double-invokes effects and re-runs ref callbacks; without the
21+
* destroy-previous-controller guard in useSafeAutoAnimate, a second
22+
* MutationObserver would linger on the same node and its remain() animation
23+
* would cancel the entrance (add) animation. We assert the user-facing outcome
24+
* (add fires, no remain) and that exactly one MutationObserver stays active on
25+
* the animated element after the cycle.
26+
*/
27+
28+
function classifyAnimateCalls(calls: any[]) {
29+
const adds: any[] = [];
30+
const remains: any[] = [];
31+
for (const call of calls) {
32+
const keyframes = call[0];
33+
if (!Array.isArray(keyframes)) {
34+
continue;
35+
}
36+
if (
37+
keyframes.some(
38+
(kf: any) => kf.opacity === 0 && typeof kf.transform === 'string' && kf.transform.includes('scale'),
39+
)
40+
) {
41+
adds.push(call);
42+
}
43+
if (keyframes.some((kf: any) => typeof kf.transform === 'string' && kf.transform.includes('translate'))) {
44+
remains.push(call);
45+
}
46+
}
47+
return { adds, remains };
48+
}
49+
50+
function AnimatedList({ showChild }: { showChild: boolean }) {
51+
return (
52+
<Animated>
53+
<div>always here</div>
54+
{showChild ? <div data-testid='new-child'>new child</div> : null}
55+
</Animated>
56+
);
57+
}
58+
59+
const flush = () => new Promise(resolve => setTimeout(resolve, 50));
60+
61+
describe('Animated under React StrictMode', () => {
62+
let animateSpy: ReturnType<typeof vi.spyOn>;
63+
64+
beforeEach(() => {
65+
animateSpy = vi
66+
.spyOn(Element.prototype, 'animate')
67+
.mockImplementation(() => ({ addEventListener: vi.fn(), cancel: vi.fn(), finished: Promise.resolve() }) as any);
68+
});
69+
70+
afterEach(() => {
71+
animateSpy.mockRestore();
72+
});
73+
74+
it('fires the add animation (and no remain) when a child is added', async () => {
75+
const { wrapper } = await createFixtures(f => {
76+
f.withUser({ email_addresses: ['test@clerk.com'] });
77+
});
78+
79+
const { rerender } = render(
80+
<StrictMode>
81+
<AnimatedList showChild={false} />
82+
</StrictMode>,
83+
{ wrapper },
84+
);
85+
86+
await flush();
87+
animateSpy.mockClear();
88+
89+
rerender(
90+
<StrictMode>
91+
<AnimatedList showChild />
92+
</StrictMode>,
93+
);
94+
95+
await flush();
96+
97+
const { adds, remains } = classifyAnimateCalls(animateSpy.mock.calls);
98+
expect(adds.length).toBeGreaterThanOrEqual(1);
99+
// remain() would only fire if a lingering second observer from the
100+
// StrictMode remount re-processed the mutation — the guard prevents it.
101+
expect(remains.length).toBe(0);
102+
});
103+
104+
it('keeps exactly one active MutationObserver on the element after the StrictMode cycle', async () => {
105+
const activeChildListObservers = new Set<MutationObserver>();
106+
const targets = new WeakMap<MutationObserver, Node>();
107+
const origObserve = MutationObserver.prototype.observe;
108+
const origDisconnect = MutationObserver.prototype.disconnect;
109+
110+
MutationObserver.prototype.observe = function (target: Node, options?: MutationObserverInit) {
111+
if (options?.childList) {
112+
activeChildListObservers.add(this);
113+
targets.set(this, target);
114+
}
115+
return origObserve.call(this, target, options);
116+
};
117+
MutationObserver.prototype.disconnect = function () {
118+
activeChildListObservers.delete(this);
119+
return origDisconnect.call(this);
120+
};
121+
122+
try {
123+
const { wrapper } = await createFixtures(f => {
124+
f.withUser({ email_addresses: ['test@clerk.com'] });
125+
});
126+
127+
render(
128+
<StrictMode>
129+
<AnimatedList showChild={false} />
130+
</StrictMode>,
131+
{ wrapper },
132+
);
133+
134+
await flush();
135+
136+
const el = screen.getByText('always here').parentElement;
137+
const activeOnEl = [...activeChildListObservers].filter(mo => targets.get(mo) === el).length;
138+
expect(activeOnEl).toBe(1);
139+
} finally {
140+
MutationObserver.prototype.observe = origObserve;
141+
MutationObserver.prototype.disconnect = origDisconnect;
142+
}
143+
});
144+
});

packages/ui/src/customizables/elementDescriptors.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,7 @@ export const APPEARANCE_KEYS = containsAllElementsConfigKeys([
465465
'profileSectionPrimaryButton',
466466
'profileSectionButtonGroup',
467467
'profilePage',
468+
'profilePageContent',
468469

469470
'formattedPhoneNumber',
470471
'formattedPhoneNumberFlag',

packages/ui/src/elements/Animated.tsx

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,37 @@
1-
import { useAutoAnimate } from '@formkit/auto-animate/react';
2-
import { cloneElement, type PropsWithChildren } from 'react';
1+
import autoAnimate from '@formkit/auto-animate';
2+
import { cloneElement, type PropsWithChildren, useCallback, useRef } from 'react';
33

44
import { useAppearance } from '@/customizables';
55

66
type AnimatedProps = PropsWithChildren<{ asChild?: boolean }>;
77

8+
type AutoAnimateController = ReturnType<typeof autoAnimate>;
9+
10+
function useSafeAutoAnimate(): [(node: HTMLElement | null) => void] {
11+
const controllerRef = useRef<AutoAnimateController | null>(null);
12+
const nodeRef = useRef<HTMLElement | null>(null);
13+
14+
const ref = useCallback((node: HTMLElement | null) => {
15+
if (node && node === nodeRef.current && controllerRef.current) {
16+
return;
17+
}
18+
if (controllerRef.current) {
19+
controllerRef.current.destroy?.();
20+
controllerRef.current = null;
21+
}
22+
nodeRef.current = node;
23+
if (node instanceof HTMLElement && typeof node.animate === 'function') {
24+
controllerRef.current = autoAnimate(node);
25+
}
26+
}, []);
27+
28+
return [ref];
29+
}
30+
831
export const Animated = (props: AnimatedProps) => {
932
const { children, asChild } = props;
1033
const { animations } = useAppearance().parsedOptions;
11-
const [parent] = useAutoAnimate();
34+
const [parent] = useSafeAutoAnimate();
1235

1336
if (asChild) {
1437
return cloneElement(children as any, { ref: animations ? parent : null });
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import React from 'react';
2+
3+
import { AppearanceContext, useAppearance } from '../customizables';
4+
import type { Elements } from '../internal/appearance';
5+
6+
export const AppearanceOverrides = ({ elements, children }: { elements: Elements; children: React.ReactNode }) => {
7+
const appearance = useAppearance();
8+
9+
const augmented = React.useMemo(() => {
10+
// position 0 is the base theme; overrides slot in immediately above it
11+
const [base, ...rest] = appearance.parsedElements;
12+
return { ...appearance, parsedElements: [base, elements, ...rest] };
13+
}, [appearance, elements]);
14+
15+
return <AppearanceContext.Provider value={{ value: augmented }}>{children}</AppearanceContext.Provider>;
16+
};

packages/ui/src/elements/ProfileCard/ProfileCardPage.tsx

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,10 @@
11
import type { PropsWithChildren } from 'react';
22

3-
import { Col } from '../../customizables';
3+
import { Col, descriptors } from '../../customizables';
44
import type { ThemableCssProp } from '../../styledSystem';
55
import { mqu } from '../../styledSystem';
66

77
type ProfileCardPageProps = PropsWithChildren<{
8-
/**
9-
* Whether to apply the standard per-page padding.
10-
* @default true
11-
*/
12-
padding?: boolean;
138
/**
149
* Whether the page should bleed past the standard padding by applying matching
1510
* negative inline margins, so children render flush with the scroll-gutter / card border.
@@ -18,7 +13,6 @@ type ProfileCardPageProps = PropsWithChildren<{
1813
bleeding?: boolean;
1914
/**
2015
* Extra styles for the page wrapper — e.g. `flex: 1` to fill the scroll box height.
21-
* Ignored when neither `padding` nor `bleeding` apply (no wrapper renders).
2216
*/
2317
sx?: ThemableCssProp;
2418
}>;
@@ -29,24 +23,19 @@ type ProfileCardPageProps = PropsWithChildren<{
2923
* Each routed page inside `UserProfile` / `OrganizationProfile` should wrap its content
3024
* in this component
3125
*/
32-
export const ProfileCardPage = ({ children, padding = true, bleeding = false, sx }: ProfileCardPageProps) => {
33-
if (!padding && !bleeding) {
34-
return <>{children}</>;
35-
}
36-
26+
export const ProfileCardPage = ({ children, bleeding = false, sx }: ProfileCardPageProps) => {
3727
return (
3828
<Col
29+
elementDescriptor={descriptors.profilePageContent}
3930
sx={[
4031
theme => ({
41-
...(padding && {
42-
paddingTop: theme.space.$7,
43-
paddingBottom: theme.space.$7,
44-
paddingInlineStart: theme.space.$8,
45-
paddingInlineEnd: theme.space.$6, //smaller because of stable scrollbar gutter on the parent
46-
[mqu.sm]: {
47-
padding: `${theme.space.$8} ${theme.space.$5}`,
48-
},
49-
}),
32+
paddingTop: theme.space.$7,
33+
paddingBottom: theme.space.$7,
34+
paddingInlineStart: theme.space.$8,
35+
paddingInlineEnd: theme.space.$6, //smaller because of stable scrollbar gutter on the parent
36+
[mqu.sm]: {
37+
padding: `${theme.space.$8} ${theme.space.$5}`,
38+
},
5039
...(bleeding && {
5140
marginInlineStart: `calc(${theme.space.$8} * -1)`,
5241
marginInlineEnd: `calc(${theme.space.$6} * -1)`,
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import type { ProfilePageId } from '@clerk/shared/types';
2+
import type { PropsWithChildren, ReactNode } from 'react';
3+
4+
import { Card } from '@/ui/elements/Card';
5+
import { Header } from '@/ui/elements/Header';
6+
7+
import type { LocalizationKey } from '../../customizables';
8+
import { Col, descriptors } from '../../customizables';
9+
import type { ThemableCssProp } from '../../styledSystem';
10+
11+
type ProfilePagePanelProps = PropsWithChildren<{
12+
pageId: ProfilePageId;
13+
titleKey: LocalizationKey;
14+
alertContent?: ReactNode;
15+
outerSx?: ThemableCssProp;
16+
}>;
17+
18+
export const ProfilePagePanel = ({ children, pageId, titleKey, alertContent, outerSx }: ProfilePagePanelProps) => {
19+
return (
20+
<Col
21+
elementDescriptor={descriptors.page}
22+
sx={outerSx ?? (t => ({ gap: t.space.$8, isolation: 'isolate' }))}
23+
>
24+
<Col
25+
elementDescriptor={descriptors.profilePage}
26+
elementId={descriptors.profilePage.setId(pageId)}
27+
>
28+
<Header.Root>
29+
<Header.Title
30+
localizationKey={titleKey}
31+
sx={t => ({ marginBottom: t.space.$4 })}
32+
textVariant='h2'
33+
/>
34+
</Header.Root>
35+
{alertContent !== undefined && <Card.Alert>{alertContent}</Card.Alert>}
36+
{children}
37+
</Col>
38+
</Col>
39+
);
40+
};

0 commit comments

Comments
 (0)