Skip to content

Commit 8893c50

Browse files
authored
fix: prevent transient HOC stacking in CompositionStore (#5047)
1 parent 1f2aa95 commit 8893c50

2 files changed

Lines changed: 42 additions & 9 deletions

File tree

packages/react-composition/src/Compose.tsx

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ export const Compose = (props: ComposeProps) => {
5252
target={targetFn.original}
5353
decorators={decorators}
5454
scope={currentScope}
55+
inherit={inherit}
5556
/>
5657
);
5758
};
@@ -64,27 +65,42 @@ function ComposeEffects({
6465
store,
6566
target,
6667
decorators,
67-
scope
68+
scope,
69+
inherit
6870
}: {
6971
store: ReturnType<typeof useCompositionStore>;
7072
target: any;
7173
decorators: Decorator<GenericComponent | GenericHook>[];
7274
scope: string;
75+
inherit: boolean;
7376
}) {
7477
const prevRef = useRef<{
7578
decorators: Decorator<GenericComponent | GenericHook>[];
7679
scope: string;
7780
} | null>(null);
7881

82+
// Tracks the decorators currently live in the store as of the last render.
83+
// Updated synchronously during render so it always reflects the most recently
84+
// registered decorators, allowing the atomic swap below to remove the right ones.
85+
const liveRef = useRef<Decorator<GenericComponent | GenericHook>[]>(decorators);
86+
87+
// On re-render with new decorators: atomically replace the old ones in the store
88+
// during render (before React commits). This ensures the store never transiently
89+
// holds both old and new HOCs — which would cause useSyncExternalStore subscribers
90+
// to render with a doubly-wrapped component and mount inner components twice.
91+
if (liveRef.current !== decorators) {
92+
store.register(target, decorators, scope, inherit, true, liveRef.current);
93+
liveRef.current = decorators;
94+
}
95+
7996
useEffect(() => {
8097
const prev = prevRef.current;
8198

82-
// On prop change: unregister old decorators.
99+
// On prop change: the render-phase atomic swap already updated the store.
100+
// Emit a non-silent notification now that effects have settled so subscribers
101+
// re-render with the final, clean composition (old HOCs fully gone).
83102
if (prev && (prev.decorators !== decorators || prev.scope !== scope)) {
84-
store.unregister(target, prev.decorators, prev.scope);
85-
// Re-register new ones (they were already registered during render,
86-
// but the idempotency check handles this).
87-
store.register(target, decorators, scope);
103+
store.notify();
88104
}
89105

90106
prevRef.current = { decorators, scope };

packages/react-composition/src/domain/CompositionStore.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ export class CompositionStore {
2222
hocs: Decorator<GenericComponent | GenericHook>[],
2323
scope = "*",
2424
inherit = false,
25-
silent = false
25+
silent = false,
26+
replaces: Decorator<GenericComponent | GenericHook>[] = []
2627
): () => void {
2728
const scopeMap: ComposedComponents = this.scopes.get(scope) || new Map();
2829
const recipe = scopeMap.get(component) || {
@@ -32,11 +33,17 @@ export class CompositionStore {
3233

3334
// Idempotent: skip if all HOCs are already registered (handles StrictMode double-render).
3435
const newHocs = hocs.filter(hoc => !recipe.hocs.includes(hoc));
35-
if (newHocs.length === 0) {
36+
if (newHocs.length === 0 && replaces.length === 0) {
3637
return () => this.unregister(component, hocs, scope);
3738
}
3839

39-
const existingHocs = [...recipe.hocs];
40+
// Atomically remove the HOCs being replaced so the store never transiently
41+
// holds both the old and new decorators at the same time. This prevents
42+
// useSyncExternalStore subscribers from seeing a doubly-wrapped component
43+
// in the window between a render-phase registration and its effect cleanup.
44+
const existingHocs = replaces.length
45+
? recipe.hocs.filter(hoc => !replaces.includes(hoc))
46+
: [...recipe.hocs];
4047
if (inherit && scope !== "*") {
4148
const globalScope = this.scopes.get("*") || new Map();
4249
const globalRecipe = globalScope.get(component) || {
@@ -124,6 +131,16 @@ export class CompositionStore {
124131
return undefined;
125132
}
126133

134+
/**
135+
* Bump version and notify listeners without changing store state.
136+
* Used after a render-phase atomic swap (silent) to inform subscribers
137+
* that the swap has settled and they should re-render with the final state.
138+
*/
139+
notify(): void {
140+
this.version++;
141+
this.notifyListeners();
142+
}
143+
127144
subscribe = (listener: Listener): (() => void) => {
128145
this.listeners.add(listener);
129146
return () => {

0 commit comments

Comments
 (0)