Skip to content

fix(react/runtime): ensure ref lifecycle events run after firstScreen#434

Merged
Yradex merged 1 commit intolynx-family:mainfrom
Yradex:lifecycle/fix-2
Apr 3, 2025
Merged

fix(react/runtime): ensure ref lifecycle events run after firstScreen#434
Yradex merged 1 commit intolynx-family:mainfrom
Yradex:lifecycle/fix-2

Conversation

@Yradex
Copy link
Copy Markdown
Collaborator

@Yradex Yradex commented Apr 1, 2025

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

  • Tests updated (or not required).
  • Documentation updated (or not required).

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 1, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: 6506f05

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@lynx-js/react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 1, 2025

Bundle Report

Changes will increase total bundle size by 89 bytes (0.02%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
@lynx-js/example-react-lynx-cjs 236.99kB 89 bytes (0.04%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: @lynx-js/example-react-lynx-cjs

Assets Changed:

Asset Name Size Change Total Size Change (%)
main.lynx.bundle 89 bytes 87.74kB 0.1%

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 1, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #434 will not alter performance

Comparing Yradex:lifecycle/fix-2 (6506f05) with main (de30a67)

Summary

✅ 1 untouched benchmarks

@Yradex Yradex force-pushed the lifecycle/fix-2 branch from a5578ec to 02ddaaa Compare April 1, 2025 06:34
@Yradex Yradex force-pushed the lifecycle/fix-2 branch from 02ddaaa to 6506f05 Compare April 1, 2025 07:10
@Yradex Yradex requested review from colinaaa and hzy April 1, 2025 07:21
@hzy hzy requested a review from Copilot April 2, 2025 04:06
Copilot AI reviewed Apr 2, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a critical issue in the React runtime’s lifecycle event handling by ensuring that ref lifecycle events are executed after firstScreen events in the background thread. Key changes include:

  • Adjusting event flushing in tt.ts and lynx-api.ts to delay lifecycle events until after firstScreen events.
  • Updating delayLifecycleEvents.ts to prioritize firstScreen events.
  • Enhancing tests to verify the correct event sequence and proper clearing of cached events.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/react/runtime/src/lynx/tt.ts Added a flush call for delayed lifecycle events with a TODO commenting on potential array merging.
packages/react/runtime/src/lynx-api.ts Conditionally flushes delayed lifecycle events based on FIRST_SCREEN_SYNC_TIMING value.
packages/react/runtime/src/lifecycle/event/delayLifecycleEvents.ts Inserts firstScreen events at the beginning to guarantee their execution order.
packages/react/runtime/src/lifecycle/destroy.ts Clears both delayed lifecycle and regular events during the destroy process.
packages/react/runtime/test/snapshot/ref.test.jsx Adds tests to verify the correct triggering and snapshot of ref lifecycle events.
packages/react/runtime/test/lifecycle/reload.test.jsx Adds a test ensuring cached events are cleared before a reload event.
.changeset/petite-bobcats-travel.md Documents the patch that ensures ref lifecycle events run after firstScreen events.

hzy
hzy reviewed Apr 2, 2025
View reviewed changes
@Yradex Yradex added this pull request to the merge queue Apr 3, 2025
Merged via the queue into lynx-family:main with commit c2523bc Apr 3, 2025
19 of 21 checks passed
@Yradex Yradex deleted the lifecycle/fix-2 branch April 3, 2025 03:32
@github-actions github-actions bot mentioned this pull request Apr 3, 2025
Merged
colinaaa pushed a commit that referenced this pull request Apr 5, 2025
@github-actions
chore: Release 2025-04-05 13:19:16 (#413)
6b210f3 
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @lynx-js/rspeedy@0.9.0

### Minor Changes

- Bundle Rspeedy with Rslib for faster start-up times.
([#395](#395))

This would be a **BREAKING CHANGE** for using [global version of
Rspeedy](https://lynxjs.org/rspeedy/cli#using-the-global-rspeedy-version).

Please ensure that you update your globally installed version of
Rspeedy:

    ```bash
    npm install --global @lynx-js/rspeedy@latest
    ```

- Bump Rsbuild v1.3.2 with Rspack v1.3.1
([#446](#446))

- **BREAKING CHANGE**: Added explicit TypeScript peer dependency
requirement of 5.1.6 - 5.8.x.
([#480](#480))

This formalizes the existing TypeScript version requirement in
`peerDependencies` (marked as optional since it is only needed for
TypeScript configurations). The actual required TypeScript version has
not changed.

Note: This may cause installation to fail if you have strict peer
dependency checks enabled.

Node.js v22.7+ users can bypass TypeScript installation using
`--experimental-transform-types` or `--experimental-strip-types` flags.
Node.js v23.6+ users don't need any flags. See [Node.js -
TypeScript](https://nodejs.org/api/typescript.html) for details.

### Patch Changes

- Support cli flag `--base` to specify the base path of the server.
([#387](#387))

- Support cli option `--environment` to specify the name of environment
to build ([#462](#462))

- Select the most appropriate network interface.
([#457](#457))

This is a port of
[webpack/webpack-dev-server#5411](webpack/webpack-dev-server#5411).

- Support Node.js v23.6+ native TypeScript.
([#481](#481))

See [Node.js - TypeScript](https://nodejs.org/api/typescript.html) for
more details.

- Support cli option `--env-mode` to specify the env mode to load the
`.env.[mode]` file.
([#453](#453))

- Support `dev.hmr` and `dev.liveReload`.
([#458](#458))

- Updated dependencies
\[[`df63722`](df63722),
[`df63722`](df63722)]:
    -   @lynx-js/chunk-loading-webpack-plugin@0.2.0

## @lynx-js/chunk-loading-webpack-plugin@0.2.0

### Minor Changes

- **BREAKING CHANGE**: Requires `@rspack/core` v1.3.0.
([#400](#400))

- **BREAKING CHANGE**: Remove the deprecated `ChunkLoadingRspackPlugin`,
use `ChunkLoadingWebpackPlugin` with `output.chunkLoading: 'lynx'`
instead. ([#400](#400))

    ```js
import { ChunkLoadingWebpackPlugin } from
"@lynx-js/chunk-loading-webpack-plugin";

    export default {
      output: {
        chunkLoading: "lynx",
      },
      plugins: [new ChunkLoadingWebpackPlugin()],
    };
    ```

## @lynx-js/react@0.106.3

### Patch Changes

- Do some global var initialize in hydrate, which fixes error like
`cannot read property '-21' of undefined` and some style issue.
([#461](#461))

- fix: ensure ref lifecycle events run after firstScreen in the
background thread
([#434](#434))

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.

## @lynx-js/qrcode-rsbuild-plugin@0.3.5

### Patch Changes

- Build with Rslib
([#396](#396))

## @lynx-js/react-rsbuild-plugin@0.9.5

### Patch Changes

- fix: add enableCSSInvalidation for encodeCSS of css HMR, this will fix
pseudo-class (such as `:active`) not working in HMR.
([#435](#435))

- Disable `module.generator.json.JSONParse` option as it increases the
bundle size of `main-thread.js`. For more detail, please see this
[issue](webpack/webpack#19319).
([#402](#402))

- Updated dependencies
\[[`3e7988f`](3e7988f),
[`7243242`](7243242)]:
    -   @lynx-js/css-extract-webpack-plugin@0.5.3
    -   @lynx-js/template-webpack-plugin@0.6.8
    -   @lynx-js/react-alias-rsbuild-plugin@0.9.5
    -   @lynx-js/react-refresh-webpack-plugin@0.3.2
    -   @lynx-js/react-webpack-plugin@0.6.10
    -   @lynx-js/web-webpack-plugin@0.6.3

## @lynx-js/web-constants@0.10.1

### Patch Changes

- feat: onNapiModulesCall function add new param: `dispatchNapiModules`,
napiModulesMap val add new param: `handleDispatch`.
([#414](#414))

Now you can use them to actively communicate to napiModules (background
thread) in onNapiModulesCall (ui thread).

-   Updated dependencies \[]:
    -   @lynx-js/web-worker-rpc@0.10.1

## @lynx-js/web-core@0.10.1

### Patch Changes

- docs: fix documents about lynx-view's properties
([#412](#412))

    Attributes should be hyphen-name: 'init-data', 'global-props'.

    now all properties has corresponding attributes.

- feat: onNapiModulesCall function add new param: `dispatchNapiModules`,
napiModulesMap val add new param: `handleDispatch`.
([#414](#414))

Now you can use them to actively communicate to napiModules (background
thread) in onNapiModulesCall (ui thread).

- Updated dependencies
\[[`1af3b60`](1af3b60)]:
    -   @lynx-js/web-constants@0.10.1
    -   @lynx-js/web-worker-runtime@0.10.1
    -   @lynx-js/web-worker-rpc@0.10.1

## @lynx-js/web-elements@0.5.3

### Patch Changes

- feat: add `layoutchange` event support for x-view and x-text
([#408](#408))

## @lynx-js/web-mainthread-apis@0.10.1

### Patch Changes

- Updated dependencies
\[[`1af3b60`](1af3b60)]:
    -   @lynx-js/web-constants@0.10.1

## @lynx-js/web-worker-runtime@0.10.1

### Patch Changes

- feat: onNapiModulesCall function add new param: `dispatchNapiModules`,
napiModulesMap val add new param: `handleDispatch`.
([#414](#414))

Now you can use them to actively communicate to napiModules (background
thread) in onNapiModulesCall (ui thread).

- Updated dependencies
\[[`1af3b60`](1af3b60)]:
    -   @lynx-js/web-constants@0.10.1
    -   @lynx-js/web-mainthread-apis@0.10.1
    -   @lynx-js/web-worker-rpc@0.10.1

## @lynx-js/css-extract-webpack-plugin@0.5.3

### Patch Changes

- Fix CSS HMR not working with nested entry name.
([#456](#456))

- fix: add enableCSSInvalidation for encodeCSS of css HMR, this will fix
pseudo-class (such as `:active`) not working in HMR.
([#435](#435))

## @lynx-js/template-webpack-plugin@0.6.8

### Patch Changes

- fix: add enableCSSInvalidation for encodeCSS of css HMR, this will fix
pseudo-class (such as `:active`) not working in HMR.
([#435](#435))

## create-rspeedy@0.9.0



## @lynx-js/react-alias-rsbuild-plugin@0.9.5



## upgrade-rspeedy@0.9.0



## @lynx-js/web-worker-rpc@0.10.1

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@hzy hzy hzy approved these changes

@colinaaa colinaaa Awaiting requested review from colinaaa

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Yradex @hzy