Skip to content

Commit f6129bb

Browse files
committed
Align \$RefreshSig\$ with React HOC-chain design; make sign idempotent
Both Babel and Oxc emit _s(memo(_c = _s(inner, key)), key) for memo-wrapped components. The status-machine in the old $RefreshSig$ would call sign with 'needsHooks' on the outer type before it had been registered, crashing with "Cannot set properties of undefined". New $RefreshSig$ implementation mirrors createSignatureFunctionForTransform from vite-plugin-react: use `typeof key === 'string'` to discriminate keyed vs body calls, always pass 'begin' for keyed calls regardless of chain depth, and collect hooks once via a didCollectHooks flag on the first body call. sign() in @prefresh/core is made idempotent on the 'begin' path (won't overwrite an inner type's getCustomHooks with an outer HOC call's undefined) and guards the 'needsHooks' path against a missing signature. Adds unit tests for $RefreshSig$ covering single components, memo-wrapped HOC chains, and the one-shot hook collection guarantee, and wires package unit tests into the GitHub Actions workflow. Fixes #610
1 parent 6b013bb commit f6129bb

6 files changed

Lines changed: 261 additions & 12 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
'@prefresh/vite': patch
3+
'@prefresh/core': patch
4+
---
5+
6+
Align $RefreshSig$ with React's HOC-chain design to fix memo-wrapped components
7+
8+
Both Babel and Oxc emit two `_s(…, key)` calls for a memo-wrapped component
9+
(`_s(memo(_c = _s(inner, key)), key)`). The previous status-machine approach
10+
mishandled the second keyed call, crashing with "Cannot set properties of
11+
undefined" because the outer type had not yet been registered when `sign` was
12+
called with `'needsHooks'`.
13+
14+
The new implementation follows the same pattern as
15+
`createSignatureFunctionForTransform` in `vite-plugin-react`:
16+
17+
- Discriminate by `typeof key === 'string'` instead of a mutable `status` variable.
18+
- Always call `sign` with `'begin'` for every keyed call, regardless of position
19+
in the HOC chain.
20+
- Track `savedType` (the innermost type) and defer `'needsHooks'` collection to
21+
the first no-argument body call, guarded by a `didCollectHooks` flag.
22+
23+
`@prefresh/core`'s `sign` is also made idempotent on the `'begin'` path so that
24+
the inner type's `getCustomHooks` is never overwritten by an outer HOC call that
25+
carries no hook information, and the `'needsHooks'` path is guarded against a
26+
missing signature entry.

.github/workflows/test.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,9 @@ jobs:
5353
run: yarn test
5454
env:
5555
CI: false
56+
57+
- name: Run package unit tests
58+
run: |
59+
node --test packages/rolldown/test/index.test.mjs
60+
node --test packages/rolldown/test/memo.test.mjs
61+
node --test packages/vite/test/refreshSig.test.mjs

packages/core/src/index.js

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ import {
1717
HOOK_CLEANUP,
1818
} from './constants';
1919
import { computeKey } from './computeKey';
20-
import { vnodesForComponent, mappedVNodes, lastSeen } from './runtime/vnodesForComponent';
20+
import {
21+
vnodesForComponent,
22+
mappedVNodes,
23+
lastSeen,
24+
} from './runtime/vnodesForComponent';
2125
import { signaturesForType } from './runtime/signaturesForType';
2226

2327
let typesById = new Map();
@@ -27,16 +31,22 @@ function sign(type, key, forceReset, getCustomHooks, status) {
2731
if (type) {
2832
let signature = signaturesForType.get(type);
2933
if (status === 'begin') {
30-
signaturesForType.set(type, {
31-
type,
32-
key,
33-
forceReset,
34-
getCustomHooks: getCustomHooks || (() => []),
35-
});
34+
// Don't overwrite an existing signature — the innermost call in an HOC
35+
// chain already carries the correct key and getCustomHooks.
36+
if (!signature) {
37+
signaturesForType.set(type, {
38+
type,
39+
key,
40+
forceReset,
41+
getCustomHooks: getCustomHooks || (() => []),
42+
});
43+
}
3644

3745
return 'needsHooks';
3846
} else if (status === 'needsHooks') {
39-
signature.fullKey = computeKey(signature);
47+
if (signature) {
48+
signature.fullKey = computeKey(signature);
49+
}
4050
}
4151
}
4252
}

packages/vite/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
],
2424
"scripts": {
2525
"lint": "eslint src",
26-
"test": "jest --clearCache && jest --runInBand --forceExit --detectOpenHandles"
26+
"test": "node --test test/refreshSig.test.mjs"
2727
},
2828
"repository": {
2929
"type": "git",

packages/vite/src/index.js

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,11 +289,27 @@ function prefreshWrapperPlugin(options = {}) {
289289
};
290290
291291
self.$RefreshSig$ = () => {
292-
let status = 'begin';
293292
let savedType;
293+
let hasCustomHooks = false;
294+
let didCollectHooks = false;
294295
return (type, key, forceReset, getCustomHooks) => {
295-
if (!savedType) savedType = type;
296-
status = self.__PREFRESH__.sign(type || savedType, key, forceReset, getCustomHooks, key ? 'begin' : status);
296+
if (typeof key === 'string') {
297+
// Keyed call: register this type. May be called multiple times
298+
// for HOC chains like _s(memo(_c = _s(inner, key)), key).
299+
if (!savedType) {
300+
savedType = type;
301+
hasCustomHooks = typeof getCustomHooks === 'function';
302+
}
303+
if (type != null) {
304+
self.__PREFRESH__.sign(type, key, forceReset, getCustomHooks, 'begin');
305+
}
306+
} else {
307+
// Body call _s() — collect custom hooks once on first render.
308+
if (!didCollectHooks && hasCustomHooks) {
309+
didCollectHooks = true;
310+
self.__PREFRESH__.sign(savedType, undefined, undefined, undefined, 'needsHooks');
311+
}
312+
}
297313
return type;
298314
};
299315
};
Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
/**
2+
* Unit tests for the $RefreshSig$ factory injected by @prefresh/vite.
3+
*
4+
* The factory is defined inline in the prelude string. These tests replicate
5+
* it verbatim and drive it against a mock __PREFRESH__.sign so we can assert
6+
* on registration order and status values without spinning up Vite.
7+
*/
8+
9+
import assert from 'node:assert/strict';
10+
import test from 'node:test';
11+
12+
// ---------------------------------------------------------------------------
13+
// Helpers
14+
// ---------------------------------------------------------------------------
15+
16+
/**
17+
* Builds a mock __PREFRESH__.sign implementation that records every call and
18+
* mirrors the real sign() state machine from @prefresh/core.
19+
*/
20+
function makeMockSign() {
21+
const signaturesForType = new Map();
22+
const calls = [];
23+
24+
function sign(type, key, forceReset, getCustomHooks, status) {
25+
calls.push({ type, key, status });
26+
if (!type) return;
27+
28+
if (status === 'begin') {
29+
if (!signaturesForType.has(type)) {
30+
signaturesForType.set(type, {
31+
type,
32+
key,
33+
forceReset,
34+
getCustomHooks: getCustomHooks || (() => []),
35+
fullKey: null,
36+
});
37+
}
38+
return 'needsHooks';
39+
} else if (status === 'needsHooks') {
40+
const sig = signaturesForType.get(type);
41+
if (sig) sig.fullKey = sig.key; // simplified computeKey
42+
}
43+
}
44+
45+
return { sign, calls, signaturesForType };
46+
}
47+
48+
/**
49+
* Builds the $RefreshSig$ factory exactly as the vite plugin injects it,
50+
* wired to a provided sign mock.
51+
*/
52+
function makeFactory(sign) {
53+
return function $RefreshSig$() {
54+
let savedType;
55+
let hasCustomHooks = false;
56+
let didCollectHooks = false;
57+
return (type, key, forceReset, getCustomHooks) => {
58+
if (typeof key === 'string') {
59+
if (!savedType) {
60+
savedType = type;
61+
hasCustomHooks = typeof getCustomHooks === 'function';
62+
}
63+
if (type != null) {
64+
sign(type, key, forceReset, getCustomHooks, 'begin');
65+
}
66+
} else {
67+
if (!didCollectHooks && hasCustomHooks) {
68+
didCollectHooks = true;
69+
sign(savedType, undefined, undefined, undefined, 'needsHooks');
70+
}
71+
}
72+
return type;
73+
};
74+
};
75+
}
76+
77+
// ---------------------------------------------------------------------------
78+
// Tests
79+
// ---------------------------------------------------------------------------
80+
81+
test('plain component: registers on keyed call and collects hooks on body call', () => {
82+
const { sign, calls, signaturesForType } = makeMockSign();
83+
const $RefreshSig$ = makeFactory(sign);
84+
85+
const getCustomHooks = () => [];
86+
const _s = $RefreshSig$();
87+
const Comp = function MyComponent() {};
88+
const key = 'abc123=';
89+
90+
// keyed call — emitted at module evaluation
91+
_s(Comp, key, false, getCustomHooks);
92+
93+
assert.equal(calls.length, 1);
94+
assert.equal(calls[0].type, Comp);
95+
assert.equal(calls[0].status, 'begin');
96+
assert.ok(signaturesForType.has(Comp));
97+
98+
// body call — emitted inside the component function, runs on first render
99+
_s();
100+
101+
assert.equal(calls.length, 2);
102+
assert.equal(calls[1].type, Comp);
103+
assert.equal(calls[1].status, 'needsHooks');
104+
105+
// subsequent renders must not re-register
106+
_s();
107+
assert.equal(calls.length, 2, 'body call is a one-shot');
108+
});
109+
110+
test('plain component without custom hooks: body call is skipped', () => {
111+
const { sign, calls } = makeMockSign();
112+
const $RefreshSig$ = makeFactory(sign);
113+
114+
const _s = $RefreshSig$();
115+
const Comp = function MyComponent() {};
116+
117+
_s(Comp, 'key='); // no getCustomHooks argument
118+
119+
_s(); // body call — should be a no-op since hasCustomHooks is false
120+
121+
assert.equal(calls.length, 1, 'only the begin call, no needsHooks');
122+
});
123+
124+
test('HOC chain (memo-wrapped): both inner and outer types are registered', () => {
125+
const { sign, calls, signaturesForType } = makeMockSign();
126+
const $RefreshSig$ = makeFactory(sign);
127+
128+
const getCustomHooks = () => [];
129+
const _s = $RefreshSig$();
130+
131+
const innerFn = function Button() {};
132+
const memoResult = { type: innerFn, $$typeof: Symbol.for('preact.memo') };
133+
const key = 'WQ9WH5eCVGcEPdUJDepp+VlX1/c=';
134+
135+
// Mirrors: const Button = _s(memo(_c = _s(innerFn, key)), key)
136+
// Evaluation is inside-out, so innerFn is registered first.
137+
_s(innerFn, key, false, getCustomHooks);
138+
_s(memoResult, key);
139+
140+
assert.equal(calls.length, 2);
141+
assert.equal(calls[0].type, innerFn);
142+
assert.equal(calls[0].status, 'begin');
143+
assert.equal(calls[1].type, memoResult);
144+
assert.equal(calls[1].status, 'begin');
145+
146+
assert.ok(signaturesForType.has(innerFn), 'inner function registered');
147+
assert.ok(signaturesForType.has(memoResult), 'memo wrapper registered');
148+
149+
// savedType is the innermost (innerFn) — body call uses it
150+
_s();
151+
assert.equal(calls.length, 3);
152+
assert.equal(calls[2].type, innerFn);
153+
assert.equal(calls[2].status, 'needsHooks');
154+
});
155+
156+
test('HOC chain: innermost registration is not overwritten by outer call', () => {
157+
const { sign, signaturesForType } = makeMockSign();
158+
const $RefreshSig$ = makeFactory(sign);
159+
160+
const getCustomHooks = () => ['useFoo'];
161+
const _s = $RefreshSig$();
162+
163+
const innerFn = function Card() {};
164+
const wrapper = { type: innerFn };
165+
const key = 'somekey=';
166+
167+
_s(innerFn, key, false, getCustomHooks); // inner: has getCustomHooks
168+
_s(wrapper, key); // outer: no getCustomHooks
169+
170+
// inner registration must preserve its getCustomHooks
171+
const sig = signaturesForType.get(innerFn);
172+
assert.equal(sig.getCustomHooks, getCustomHooks);
173+
});
174+
175+
test('multiple renders: body call only triggers needsHooks once', () => {
176+
const { sign, calls } = makeMockSign();
177+
const $RefreshSig$ = makeFactory(sign);
178+
179+
const getCustomHooks = () => [];
180+
const _s = $RefreshSig$();
181+
const Comp = function Foo() {};
182+
183+
_s(Comp, 'k=', false, getCustomHooks);
184+
185+
_s(); // render 1
186+
_s(); // render 2
187+
_s(); // render 3
188+
189+
const needsHooksCalls = calls.filter(c => c.status === 'needsHooks');
190+
assert.equal(needsHooksCalls.length, 1);
191+
});

0 commit comments

Comments
 (0)