Skip to content

fix(react): run all pending renderComponent before hydrate #1438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(react): run all pending renderComponent before hydrate #1438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 112 additions & 0 deletions packages/react/runtime/__test__/lifecycle/updateData.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -864,3 +864,115 @@ describe('triggerDataUpdated when jsReady is enabled', () => {
}
});
});

describe('flush pending `renderComponent` before hydrate', () => {
beforeEach(() => {
globalThis.__FIRST_SCREEN_SYNC_TIMING__ = 'jsReady';
});

afterEach(() => {
globalThis.__FIRST_SCREEN_SYNC_TIMING__ = 'immediately';
});

it('should not send triggerDataUpdated when updateData after hydration', async function() {
function Comp() {
const initData = useInitData();

return <text>{initData.msg}</text>;
}

// main thread render
{
__root.__jsx = <Comp />;
renderPage({ msg: 'init' });
expect(__root.__element_root).toMatchInlineSnapshot(`
<page
cssId="default-entry-from-native:0"
>
<text>
<raw-text
text="init"
/>
</text>
</page>
`);
}

// main thread updatePage
{
__root.__jsx = <Comp />;
updatePage({ msg: 'update' });
expect(__root.__element_root).toMatchInlineSnapshot(`
<page
cssId="default-entry-from-native:0"
>
<text>
<raw-text
text="update"
/>
</text>
</page>
`);
}

// background render
{
globalEnvManager.switchToBackground();
render(<Comp />, __root);
Copy link
Collaborator

@upupming upupming Aug 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This render will use update because we share lynx.__initData across 2 threads in test framework.

}

// LifecycleConstant.jsReady
{
globalEnvManager.switchToMainThread();
rLynxJSReady();
}

// background updateCardData
{
globalEnvManager.switchToBackground();
lynxCoreInject.tt.updateCardData({ msg: 'update' });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we assert that setState is called when updateCardData is executed?

}

// hydrate
Comment on lines +930 to +936
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Strengthen assertion: prove no background rLynxChange is dispatched pre-hydration

To validate the PR’s intent (pending updates are flushed into hydration rather than sent early), explicitly assert that no rLynxChange call is made from updateCardData before hydration.

     // background updateCardData
     {
       globalEnvManager.switchToBackground();
-      lynxCoreInject.tt.updateCardData({ msg: 'update' });
+      lynx.getNativeApp().callLepusMethod.mockClear();
+      lynxCoreInject.tt.updateCardData({ msg: 'update' });
+      await waitSchedule();
+      expect(lynx.getNativeApp().callLepusMethod).toHaveBeenCalledTimes(0);
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// background updateCardData
{
globalEnvManager.switchToBackground();
lynxCoreInject.tt.updateCardData({ msg: 'update' });
}
// hydrate
// background updateCardData
{
globalEnvManager.switchToBackground();
lynx.getNativeApp().callLepusMethod.mockClear();
lynxCoreInject.tt.updateCardData({ msg: 'update' });
await waitSchedule();
expect(lynx.getNativeApp().callLepusMethod).toHaveBeenCalledTimes(0);
}
// hydrate

{
globalEnvManager.switchToBackground();
// LifecycleConstant.firstScreen
lynxCoreInject.tt.OnLifecycleEvent(...globalThis.__OnLifecycleEvent.mock.calls[0]);
Copy link
Collaborator

@upupming upupming Aug 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we assert that process is executed and state is updated? we can import process from preact and spy on it.

}

// rLynxChange
{
globalEnvManager.switchToMainThread();
globalThis.__OnLifecycleEvent.mockClear();
const rLynxChange = lynx.getNativeApp().callLepusMethod.mock.calls[0];
globalThis[rLynxChange[0]](rLynxChange[1]);
expect(rLynxChange[1]).toMatchInlineSnapshot(`
{
"data": "{"patchList":[{"snapshotPatch":[],"id":24}]}",
"patchOptions": {
"isHydration": true,
"pipelineOptions": {
"dsl": "reactLynx",
"needTimestamps": true,
"pipelineID": "pipelineID",
"pipelineOrigin": "reactLynxHydrate",
"stage": "hydrate",
},
"reloadVersion": 0,
},
}
`);
expect(__root.__element_root).toMatchInlineSnapshot(`
<page
cssId="default-entry-from-native:0"
>
<text>
<raw-text
text="update"
/>
</text>
</page>
`);
}
});
});
Comment on lines +868 to +978
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Test logic contradicts the test name and description.

The test is named "should not send triggerDataUpdated when updateData after hydration", but the actual test sequence shows updateCardData being called before hydration (line 909), not after. The hydration occurs later on line 916.

This creates confusion about what the test is actually verifying and may not align with the PR objective of ensuring pending renderComponent calls are executed before hydration.

Consider one of these fixes:

Option 1: Fix the test name to match the actual logic:

- it('should not send triggerDataUpdated when updateData after hydration', async function() {
+ it('should not send triggerDataUpdated when updateData before hydration', async function() {

Option 2: Fix the test logic to match the name:
Move the updateCardData call to occur after the hydration step if you want to test the "after hydration" scenario.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
describe('flush pending `renderComponent` before hydrate', () => {
beforeEach(() => {
globalThis.__FIRST_SCREEN_SYNC_TIMING__ = 'jsReady';
});
afterEach(() => {
globalThis.__FIRST_SCREEN_SYNC_TIMING__ = 'immediately';
});
it('should not send triggerDataUpdated when updateData after hydration', async function() {
function Comp() {
const initData = useInitData();
return <text>{initData.msg}</text>;
}
// main thread render
{
__root.__jsx = <Comp />;
renderPage({ msg: 'init' });
expect(__root.__element_root).toMatchInlineSnapshot(`
<page
cssId="default-entry-from-native:0"
>
<text>
<raw-text
text="init"
/>
</text>
</page>
`);
}
// main thread updatePage
{
__root.__jsx = <Comp />;
updatePage({ msg: 'update' });
expect(__root.__element_root).toMatchInlineSnapshot(`
<page
cssId="default-entry-from-native:0"
>
<text>
<raw-text
text="update"
/>
</text>
</page>
`);
}
// background render
{
globalEnvManager.switchToBackground();
render(<Comp />, __root);
}
// LifecycleConstant.jsReady
{
globalEnvManager.switchToMainThread();
rLynxJSReady();
}
// background updateCardData
{
globalEnvManager.switchToBackground();
lynxCoreInject.tt.updateCardData({ msg: 'update' });
}
// hydrate
{
globalEnvManager.switchToBackground();
// LifecycleConstant.firstScreen
lynxCoreInject.tt.OnLifecycleEvent(...globalThis.__OnLifecycleEvent.mock.calls[0]);
}
// rLynxChange
{
globalEnvManager.switchToMainThread();
globalThis.__OnLifecycleEvent.mockClear();
const rLynxChange = lynx.getNativeApp().callLepusMethod.mock.calls[0];
globalThis[rLynxChange[0]](rLynxChange[1]);
expect(rLynxChange[1]).toMatchInlineSnapshot(`
{
"data": "{"patchList":[{"snapshotPatch":[],"id":24}]}",
"patchOptions": {
"isHydration": true,
"pipelineOptions": {
"dsl": "reactLynx",
"needTimestamps": true,
"pipelineID": "pipelineID",
"pipelineOrigin": "reactLynxHydrate",
"stage": "hydrate",
},
"reloadVersion": 0,
},
}
`);
expect(__root.__element_root).toMatchInlineSnapshot(`
<page
cssId="default-entry-from-native:0"
>
<text>
<raw-text
text="update"
/>
</text>
</page>
`);
}
});
});
it('should not send triggerDataUpdated when updateData before hydration', async function() {
🤖 Prompt for AI Agents 
In packages/react/runtime/__test__/lifecycle/updateData.test.jsx between lines
844 and 954, the test named "should not send triggerDataUpdated when updateData
after hydration" incorrectly calls updateCardData before hydration,
contradicting the test name and intent. To fix this, either rename the test to
reflect that updateCardData is called before hydration or move the
updateCardData call to after the hydration step to properly test the "after
hydration" scenario as described.

Loading
Loading