Skip to content

Commit c2523bc

Browse files
authored
fix(react/runtime): ensure ref lifecycle events run after firstScreen (#434)
## Summary This PR fixes a critical issue in the React runtime's lifecycle event handling, specifically in the background thread async render mode. The problem was that ref lifecycle events were being triggered before firstScreen events, which could lead to undefined refs when components try to access them. The root cause was in the event ordering in the background thread. In the async render mode, we need to ensure that firstScreen events complete before ref events are processed, as components may depend on the firstScreen state being fully initialized. The fix ensures proper event ordering by: 1. Modifying the lifecycle event delay mechanism 2. Updating the ref event handling to wait for firstScreen completion 3. Adding comprehensive test cases to verify the correct event sequence This change improves the reliability of component initialization and ref handling in the background thread, preventing potential undefined ref errors that could occur during component mounting. ## Checklist - [x] Tests updated (or not required). - [x] Documentation updated (or not required).
1 parent e8da709 commit c2523bc

File tree

7 files changed

+147
-7
lines changed

7 files changed

+147
-7
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@lynx-js/react": patch
3+
---
4+
5+
fix: ensure ref lifecycle events run after firstScreen in the background thread
6+
7+
This patch fixes an issue where ref lifecycle events were running before firstScreen events in the background thread async render mode, which could cause refs to be undefined when components try to access them.

packages/react/runtime/__test__/lifecycle/reload.test.jsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi }
99
import { BasicBG, ListBG, ListConditionalBG, ViewBG, setObj, setStr } from './reloadBG';
1010
import { BasicMT, ListConditionalMT, ListMT, ViewMT } from './reloadMT';
1111
import { root } from '../../src/index';
12+
import { delayedEvents, delayedPublishEvent } from '../../src/lifecycle/event/delayEvents';
1213
import { replaceCommitHook } from '../../src/lifecycle/patch/commit';
1314
import { injectUpdateMainThread } from '../../src/lifecycle/patch/updateMainThread';
15+
import { reloadBackground } from '../../src/lifecycle/reload';
1416
import { __root } from '../../src/root';
1517
import { setupPage } from '../../src/snapshot';
1618
import { globalEnvManager } from '../utils/envManager';
@@ -1882,4 +1884,11 @@ describe('firstScreenSyncTiming - jsReady', () => {
18821884
lynx.getNativeApp().callLepusMethod.mockClear();
18831885
}
18841886
});
1887+
1888+
it('should clear cached events before reload when js not ready', async function() {
1889+
delayedPublishEvent('bindEvent:tap', 'test');
1890+
expect(delayedEvents.length).toBe(1);
1891+
reloadBackground({});
1892+
expect(delayedEvents.length).toBe(0);
1893+
});
18851894
});

packages/react/runtime/__test__/snapshot/ref.test.jsx

Lines changed: 103 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
*/
66
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
77

8-
import { Component, createRef, useState } from '../../src/index';
8+
import { Component, createRef, root, useState } from '../../src/index';
9+
import { delayedLifecycleEvents } from '../../src/lifecycle/event/delayLifecycleEvents';
910
import { replaceCommitHook } from '../../src/lifecycle/patch/commit';
1011
import { injectUpdateMainThread } from '../../src/lifecycle/patch/updateMainThread';
1112
import { renderBackground as render } from '../../src/lifecycle/render';
@@ -1650,4 +1651,105 @@ describe('element ref in list', () => {
16501651
`);
16511652
}
16521653
});
1654+
1655+
it('when __FIRST_SCREEN_SYNC_TIMING__ is jsReady', async function() {
1656+
globalThis.__FIRST_SCREEN_SYNC_TIMING__ = 'jsReady';
1657+
const refs = [createRef(), createRef(), createRef()];
1658+
const signs = [0, 0, 0];
1659+
1660+
class ListItem extends Component {
1661+
render() {
1662+
return <view ref={this.props._ref}></view>;
1663+
}
1664+
}
1665+
1666+
class Comp extends Component {
1667+
render() {
1668+
return (
1669+
<list>
1670+
{[0, 1, 2].map((index) => {
1671+
return (
1672+
<list-item item-key={index}>
1673+
<ListItem _ref={refs[index]}></ListItem>
1674+
</list-item>
1675+
);
1676+
})}
1677+
</list>
1678+
);
1679+
}
1680+
}
1681+
1682+
// main thread render
1683+
{
1684+
__root.__jsx = <Comp />;
1685+
renderPage();
1686+
}
1687+
1688+
// list render item 1 & 2
1689+
{
1690+
signs[0] = elementTree.triggerComponentAtIndex(__root.childNodes[0].__elements[0], 0);
1691+
expect(globalThis.__OnLifecycleEvent).toHaveBeenCalledTimes(1);
1692+
1693+
globalEnvManager.switchToBackground();
1694+
lynxCoreInject.tt.OnLifecycleEvent(...globalThis.__OnLifecycleEvent.mock.calls[0]);
1695+
globalThis.__OnLifecycleEvent.mockClear();
1696+
expect(delayedLifecycleEvents).toMatchInlineSnapshot(`
1697+
[
1698+
[
1699+
"rLynxRef",
1700+
{
1701+
"commitTaskId": undefined,
1702+
"refPatch": "{"-4:0:":62}",
1703+
},
1704+
],
1705+
]
1706+
`);
1707+
}
1708+
1709+
// background render
1710+
{
1711+
globalEnvManager.switchToBackground();
1712+
root.render(<Comp />, __root);
1713+
expect(lynx.getNativeApp().callLepusMethod).toHaveBeenCalledTimes(1);
1714+
expect(lynx.getNativeApp().callLepusMethod.mock.calls[0]).toMatchInlineSnapshot(`
1715+
[
1716+
"rLynxJSReady",
1717+
{},
1718+
]
1719+
`);
1720+
globalEnvManager.switchToMainThread();
1721+
const rLynxJSReady = lynx.getNativeApp().callLepusMethod.mock.calls[0];
1722+
globalThis[rLynxJSReady[0]](rLynxJSReady[1]);
1723+
lynx.getNativeApp().callLepusMethod.mockClear();
1724+
expect(globalThis.__OnLifecycleEvent).toHaveBeenCalledTimes(1);
1725+
expect(globalThis.__OnLifecycleEvent.mock.calls).toMatchInlineSnapshot(`
1726+
[
1727+
[
1728+
[
1729+
"rLynxFirstScreen",
1730+
{
1731+
"jsReadyEventIdSwap": {},
1732+
"refPatch": "{}",
1733+
"root": "{"id":-1,"type":"root","children":[{"id":-2,"type":"__Card__:__snapshot_a94a8_test_24","children":[{"id":-3,"type":"__Card__:__snapshot_a94a8_test_25","values":[{"item-key":0}],"children":[{"id":-4,"type":"__Card__:__snapshot_a94a8_test_23","values":["-4:0:"]}]},{"id":-5,"type":"__Card__:__snapshot_a94a8_test_25","values":[{"item-key":1}],"children":[{"id":-6,"type":"__Card__:__snapshot_a94a8_test_23","values":["-6:0:"]}]},{"id":-7,"type":"__Card__:__snapshot_a94a8_test_25","values":[{"item-key":2}],"children":[{"id":-8,"type":"__Card__:__snapshot_a94a8_test_23","values":["-8:0:"]}]}]}]}",
1734+
},
1735+
],
1736+
],
1737+
]
1738+
`);
1739+
}
1740+
1741+
{
1742+
globalEnvManager.switchToBackground();
1743+
lynxCoreInject.tt.OnLifecycleEvent(...globalThis.__OnLifecycleEvent.mock.calls[0]);
1744+
globalThis.__OnLifecycleEvent.mockClear();
1745+
1746+
expect(refs[0].current).toMatchInlineSnapshot(`
1747+
{
1748+
"selectUniqueID": [Function],
1749+
"uid": 62,
1750+
}
1751+
`);
1752+
}
1753+
globalThis.__FIRST_SCREEN_SYNC_TIMING__ = 'immediately';
1754+
});
16531755
});

packages/react/runtime/src/lifecycle/destroy.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// Licensed under the Apache License Version 2.0 that can be found in the
33
// LICENSE file in the root directory of this source tree.
44
import { __root } from '../root.js';
5+
import { delayedEvents } from './event/delayEvents.js';
6+
import { delayedLifecycleEvents } from './event/delayLifecycleEvents.js';
57
import { globalCommitTaskMap } from './patch/commit.js';
68
import { renderBackground } from './render.js';
79

@@ -16,7 +18,12 @@ function destroyBackground(): void {
1618
task();
1719
});
1820
globalCommitTaskMap.clear();
19-
21+
// Clear delayed events which should not be executed after destroyed.
22+
// This is important when the page is performing a reload.
23+
delayedLifecycleEvents.length = 0;
24+
if (delayedEvents) {
25+
delayedEvents.length = 0;
26+
}
2027
if (__PROFILE__) {
2128
console.profileEnd();
2229
}

packages/react/runtime/src/lifecycle/event/delayLifecycleEvents.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,18 @@
1+
import { LifecycleConstant } from '../../lifecycleConstant.js';
2+
13
const delayedLifecycleEvents: [type: string, data: any][] = [];
24

35
function delayLifecycleEvent(type: string, data: any): void {
4-
delayedLifecycleEvents.push([type, data]);
6+
// We need to ensure that firstScreen events are executed before other events.
7+
// This is because firstScreen events are used to initialize the dom tree,
8+
// and other events depend on the dom tree being fully constructed.
9+
// There might be some edge cases where ctx cannot be found in `ref` lifecycle event,
10+
// and they should be ignored safely.
11+
if (type === LifecycleConstant.firstScreen) {
12+
delayedLifecycleEvents.unshift([type, data]);
13+
} else {
14+
delayedLifecycleEvents.push([type, data]);
15+
}
516
}
617

718
/**

packages/react/runtime/src/lynx-api.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,13 @@ export const root: Root = {
8787
} else {
8888
__root.__jsx = jsx;
8989
renderBackground(jsx, __root as any);
90-
if (__FIRST_SCREEN_SYNC_TIMING__ === 'immediately') {}
91-
else {
90+
if (__FIRST_SCREEN_SYNC_TIMING__ === 'immediately') {
91+
// This is for cases where `root.render()` is called asynchronously,
92+
// `firstScreen` message might have been reached.
93+
flushDelayedLifecycleEvents();
94+
} else {
9295
lynx.getNativeApp().callLepusMethod(LifecycleConstant.jsReady, {});
9396
}
94-
95-
flushDelayedLifecycleEvents();
9697
}
9798
},
9899
registerDataProcessors: (dataProcessorDefinition: DataProcessorDefinition): void => {

packages/react/runtime/src/lynx/tt.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ async function onLifecycleEventImpl(type: string, data: any): Promise<void> {
131131
}
132132
markTiming(PerformanceTimingKeys.diff_vdom_end);
133133

134+
// TODO: It seems `delayedEvents` and `delayedLifecycleEvents` should be merged into one array to ensure the proper order of events.
135+
flushDelayedLifecycleEvents();
134136
if (delayedEvents) {
135137
delayedEvents.forEach((args) => {
136138
const [handlerName, data] = args;
@@ -144,6 +146,7 @@ async function onLifecycleEventImpl(type: string, data: any): Promise<void> {
144146
});
145147
delayedEvents.length = 0;
146148
}
149+
147150
lynxCoreInject.tt.publishEvent = publishEvent;
148151
lynxCoreInject.tt.publicComponentEvent = publicComponentEvent;
149152

0 commit comments

Comments
 (0)