diff --git a/.changeset/fix-react-ref-before-hydration.md b/.changeset/fix-react-ref-before-hydration.md new file mode 100644 index 0000000000..c304941d00 --- /dev/null +++ b/.changeset/fix-react-ref-before-hydration.md @@ -0,0 +1,5 @@ +--- +"@lynx-js/react": patch +--- + +Fix ref callbacks not being cleaned up or re-applied correctly when the ref at the same element slot changes across rerenders that happen before hydration (e.g. a `useEffect` triggering `setState` during the initial background render). diff --git a/packages/react/runtime/__test__/snapshot/ref.test.jsx b/packages/react/runtime/__test__/snapshot/ref.test.jsx index 3046fee74a..e1730fc4a9 100644 --- a/packages/react/runtime/__test__/snapshot/ref.test.jsx +++ b/packages/react/runtime/__test__/snapshot/ref.test.jsx @@ -1979,6 +1979,179 @@ describe('ui operations', () => { }); }); +describe('applyRef before hydration', () => { + it('ref is changed across rerenders before hydration', async function() { + const oldCb = vi.fn(); + const newCb = vi.fn(); + + function App({ cb }) { + return ; + } + + globalEnvManager.switchToBackground(); + render(, __root); + render(, __root); + + expect(oldCb).toHaveBeenCalledTimes(2); + expect(oldCb.mock.calls[0][0]).toBeInstanceOf(RefProxy); + expect(oldCb.mock.calls[1][0]).toBeNull(); + + expect(newCb).toHaveBeenCalledTimes(1); + expect(newCb.mock.calls[0][0]).toBeInstanceOf(RefProxy); + }); + + it('ref becomes null on rerender before hydration', async function() { + const cb = vi.fn(); + + function App({ useRef }) { + return ; + } + + globalEnvManager.switchToBackground(); + render(, __root); + render(, __root); + + expect(cb).toHaveBeenCalledTimes(2); + expect(cb.mock.calls[0][0]).toBeInstanceOf(RefProxy); + expect(cb.mock.calls[1][0]).toBeNull(); + }); + + it('ref is added on rerender before hydration', async function() { + const cb = vi.fn(); + + function App({ useRef }) { + return ; + } + + globalEnvManager.switchToBackground(); + render(, __root); + render(, __root); + + expect(cb).toHaveBeenCalledTimes(1); + expect(cb.mock.calls[0][0]).toBeInstanceOf(RefProxy); + }); + + it('spread ref is removed on rerender before hydration', async function() { + const cb = vi.fn(); + + function App({ withRef }) { + return ; + } + + globalEnvManager.switchToBackground(); + render(, __root); + render(, __root); + + expect(cb).toHaveBeenCalledTimes(2); + expect(cb.mock.calls[0][0]).toBeInstanceOf(RefProxy); + expect(cb.mock.calls[1][0]).toBeNull(); + }); + + it('spread ref is added on rerender before hydration', async function() { + const cb = vi.fn(); + + function App({ withRef }) { + return ; + } + + globalEnvManager.switchToBackground(); + render(, __root); + render(, __root); + + expect(cb).toHaveBeenCalledTimes(1); + expect(cb.mock.calls[0][0]).toBeInstanceOf(RefProxy); + }); + + it('object ref (createRef) is changed across rerenders before hydration', async function() { + const oldRef = createRef(); + const newRef = createRef(); + + function App({ r }) { + return ; + } + + globalEnvManager.switchToBackground(); + render(, __root); + expect(oldRef.current).toBeInstanceOf(RefProxy); + expect(newRef.current).toBeNull(); + + render(, __root); + expect(oldRef.current).toBeNull(); + expect(newRef.current).toBeInstanceOf(RefProxy); + }); + + it('object ref (createRef) becomes null on rerender before hydration', async function() { + const ref = createRef(); + + function App({ useRef }) { + return ; + } + + globalEnvManager.switchToBackground(); + render(, __root); + expect(ref.current).toBeInstanceOf(RefProxy); + + render(, __root); + expect(ref.current).toBeNull(); + }); + + it('object ref (createRef) is added on rerender before hydration', async function() { + const ref = createRef(); + + function App({ useRef }) { + return ; + } + + globalEnvManager.switchToBackground(); + render(, __root); + expect(ref.current).toBeNull(); + + render(, __root); + expect(ref.current).toBeInstanceOf(RefProxy); + }); + + it('same ref callback in spread form should not be re-invoked', async function() { + const cb = vi.fn(); + + function App() { + return ; + } + + globalEnvManager.switchToBackground(); + render(, __root); + render(, __root); + + expect(cb).toHaveBeenCalledTimes(1); + expect(cb.mock.calls[0][0]).toBeInstanceOf(RefProxy); + }); + + it('three consecutive rerenders before hydration clean up intermediate refs', async function() { + const cb1 = vi.fn(); + const cb2 = vi.fn(); + const cb3 = vi.fn(); + + function App({ cb }) { + return ; + } + + globalEnvManager.switchToBackground(); + render(, __root); + render(, __root); + render(, __root); + + expect(cb1).toHaveBeenCalledTimes(2); + expect(cb1.mock.calls[0][0]).toBeInstanceOf(RefProxy); + expect(cb1.mock.calls[1][0]).toBeNull(); + + expect(cb2).toHaveBeenCalledTimes(2); + expect(cb2.mock.calls[0][0]).toBeInstanceOf(RefProxy); + expect(cb2.mock.calls[1][0]).toBeNull(); + + expect(cb3).toHaveBeenCalledTimes(1); + expect(cb3.mock.calls[0][0]).toBeInstanceOf(RefProxy); + }); +}); + describe('runDelayedUiOps helper', () => { it('should reset shouldDelayUiOps when no tasks queued', () => { // flush any queued tasks from previous tests diff --git a/packages/react/runtime/src/snapshot/snapshot/backgroundSnapshot.ts b/packages/react/runtime/src/snapshot/snapshot/backgroundSnapshot.ts index 7b34699ee0..cef524af06 100644 --- a/packages/react/runtime/src/snapshot/snapshot/backgroundSnapshot.ts +++ b/packages/react/runtime/src/snapshot/snapshot/backgroundSnapshot.ts @@ -13,7 +13,7 @@ import type { Worklet } from '@lynx-js/react/worklet-runtime/bindings'; import { snapshotManager } from './definition.js'; import type { Snapshot } from './definition.js'; import { DynamicPartType } from './dynamicPartType.js'; -import { applyRef, clearQueuedRefs, queueRefAttrUpdate } from './ref.js'; +import { applyRef, clearQueuedRefs, getRefFromValue, queueRefAttrUpdate } from './ref.js'; import type { Ref } from './ref.js'; import { snapshotCreatorMap } from './snapshot.js'; import { hydrationMap } from './snapshotInstanceHydrationMap.js'; @@ -362,14 +362,11 @@ export class BackgroundSnapshotInstance { } } else { this.__snapshot_def.refAndSpreadIndexes?.forEach((index) => { + // In first render, this.__values is undefined. + // In next rerenders before hydration, this.__values is not undefined. + const oldValue: unknown = this.__values?.[index]; const v = (value as unknown[])[index]; - if (v && (typeof v === 'object' || typeof v === 'function')) { - if ('__spread' in v && 'ref' in v) { - queueRefAttrUpdate(null, v.ref as Ref, this.__id, index); - } else if ('__ref' in v) { - queueRefAttrUpdate(null, v as Ref, this.__id, index); - } - } + queueRefAttrUpdate(getRefFromValue(oldValue), getRefFromValue(v), this.__id, index); }); } this.__values = value as unknown[]; diff --git a/packages/react/runtime/src/snapshot/snapshot/ref.ts b/packages/react/runtime/src/snapshot/snapshot/ref.ts index b082b79616..25bcb0b39c 100644 --- a/packages/react/runtime/src/snapshot/snapshot/ref.ts +++ b/packages/react/runtime/src/snapshot/snapshot/ref.ts @@ -80,6 +80,19 @@ function updateRef( } } +function getRefFromValue(val: unknown): Ref | null { + if (!val || (typeof val !== 'object' && typeof val !== 'function')) { + return null; + } + if ('__spread' in val && 'ref' in val) { + return ((val as { ref?: Ref | null }).ref) ?? null; + } + if ('__ref' in val) { + return val as Ref; + } + return null; +} + function transformRef(ref: unknown): Ref | null | undefined { if (ref === undefined || ref === null) { return ref; @@ -136,4 +149,14 @@ function clearQueuedRefs(): void { /** * @internal */ -export { queueRefAttrUpdate, updateRef, unref, transformRef, applyRef, applyQueuedRefs, clearQueuedRefs, type Ref }; +export { + queueRefAttrUpdate, + updateRef, + unref, + transformRef, + applyRef, + applyQueuedRefs, + clearQueuedRefs, + getRefFromValue, + type Ref, +}; diff --git a/packages/react/testing-library/src/__tests__/ref.test.jsx b/packages/react/testing-library/src/__tests__/ref.test.jsx index d1ca395752..5b94cbd2bc 100644 --- a/packages/react/testing-library/src/__tests__/ref.test.jsx +++ b/packages/react/testing-library/src/__tests__/ref.test.jsx @@ -1,6 +1,6 @@ -import { createRef, Component, useState } from '@lynx-js/react'; +import { createRef, Component, useState, useEffect, useRef } from '@lynx-js/react'; import { render } from '..'; -import { expect, vi } from 'vitest'; +import { expect, vi, describe, it } from 'vitest'; import { act } from 'preact/test-utils'; describe('component ref', () => { @@ -483,3 +483,91 @@ describe('element ref', () => { }); }); }); + +describe('applyRef before hydration', () => { + it('rerender with same ref callback should not invoke ref callback', () => { + const refCallback = vi.fn(); + let bump; + + function App() { + const [, setTick] = useState(0); + bump = () => setTick(t => t + 1); + + useEffect(() => { + // This will trigger a rerender before hydration + bump(); + }, []); + + return ; + } + + render(); + expect(refCallback).toHaveBeenCalledTimes(1); + expect(refCallback.mock.calls[0][0]).toMatchObject({ + refAttr: expect.any(Array), + }); + }); + + const forms = [ + 'normal', + 'spread', + ]; + forms.forEach((key) => { + forms.forEach((key2) => { + it(`rerender when ref is changed from ${key} to ${key2}`, () => { + const oldCb = vi.fn(); + const newCb = vi.fn(); + let bump; + + function App() { + const [tick, setTick] = useState(0); + bump = () => setTick(t => t + 1); + + useEffect(() => { + // This will trigger a rerender before hydration + bump(); + }, []); + + const isFirst = tick === 0; + const ref = isFirst ? oldCb : newCb; + const form = isFirst ? key : key2; + + if (form === 'spread') { + return ; + } + return ; + } + + render(); + + expect(oldCb).toHaveBeenCalledTimes(2); + expect(oldCb.mock.calls[0][0]).toMatchObject({ + refAttr: expect.any(Array), + }); + expect(oldCb.mock.calls[1][0]).toBeNull(); + + expect(newCb).toHaveBeenCalledTimes(1); + expect(newCb.mock.calls[0][0]).toMatchObject({ + refAttr: expect.any(Array), + }); + }); + }); + }); + + it('useRef + useEffect + setState host capture is stable (portal-host pattern)', () => { + const seenHosts = vi.fn(); + + function App() { + const hostRef = useRef(null); + const [host, setHost] = useState(null); + useEffect(() => { + setHost(hostRef.current); + }, []); + if (host) seenHosts(host); + return ; + } + + render(); + expect(seenHosts).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/react/testing-library/src/pure.jsx b/packages/react/testing-library/src/pure.jsx index a8c5aa96bc..5780dec49e 100644 --- a/packages/react/testing-library/src/pure.jsx +++ b/packages/react/testing-library/src/pure.jsx @@ -71,8 +71,9 @@ export function render( globalThis.lynxTestingEnv.switchToBackgroundThread(); act(() => { preactRender(compBackgroundThread, __root); - flushDelayedLifecycleEvents(); }); + // `OnLifecycleEvent::rLynxFirstScreen` should happen after `useEffect` + flushDelayedLifecycleEvents(); } return {