Skip to content

fix(react/runtime): resolve race condition in runWithForce#707

Merged
colinaaa merged 2 commits intolynx-family:mainfrom
colinaaa:colin/0507/run-with-force
May 9, 2025
Merged

fix(react/runtime): resolve race condition in runWithForce#707
colinaaa merged 2 commits intolynx-family:mainfrom
colinaaa:colin/0507/run-with-force

Conversation

@colinaaa
Copy link
Collaborator

@colinaaa colinaaa commented May 7, 2025

Summary

The options[DIFF] may be overridden during the unmount process:

options[DIFF] = (vnode: VNode) => {
// A new diff indicates that the unmounting process of parentVNode is finished.
parentVNode = undefined;
options[DIFF] = oldDiff;
oldDiff?.(vnode);
};

This would conflict with the overwriting of options[DIFF] within runWithForce():

options[DIFF] = (vnode: VNode) => {

In a real-world scenario, consider simultaneously updating GlobalProps and states.

  • First tick:
    • We have options[DIFF] === oldDiff at the beginning.
    • updateGlobalProps
      • We still have options[DIFF] === diffUnmount
      • A Promise.resolve().then() is called to enqueue a runWithForce update.
    • renderComponent
      • We set options[DIFF] to diffUnmount
      • when there is a state update that causes a JSX being removed (options.unmount will be called)
  • Second tick:
    • runWithForce is called
      • We set options[DIFF] to diffRunWithForce
    • render()
      • options[DIFF] would be called in the order of diffRunWithForce -> diffUnmount -> oldDiff
      • In diffUnmount, the options[DIFF] would be reset to oldDiff
      • But the options[DIFFED] is still overwritten. And the vnode[COMPONENT] would be set to undefined for every following vnode. 👈 This is what we are fixing in the patch
        options[DIFFED] = (vnode: VNode) => {
        if (oldDiffed) {
        oldDiffed(vnode);
        }
        // delete is a reverse operation of previous `Object.defineProperty`
        delete vnode[COMPONENT];
        // restore
        vnode[COMPONENT] = m.get(vnode);
        };
    • runWithForce ended
      • We set options[DIFF] to diffUnmount

We are not addressing the value of options[DIFF] in this patch, as it does not currently cause incorrect behavior. However, we are resolving the vnode[COMPONENT] issue, as it leads to incorrect functionality in hooks and diff operations.

Checklist

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

@changeset-bot
Copy link

changeset-bot bot commented May 7, 2025

🦋 Changeset detected

Latest commit: 8a85ec4

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

@colinaaa colinaaa requested a review from hzy May 7, 2025 15:46
@codecov
Copy link

codecov bot commented May 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@codecov
Copy link

codecov bot commented May 7, 2025

Bundle Report

Changes will increase total bundle size by 3.19kB (0.5%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
@lynx-js/example-react-lynx-cjs 320.98kB 3.19kB (1.0%) ⬆️

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 1.62kB 89.75kB 1.83%
.rspeedy/main/main-thread.js 399 bytes 40.46kB 1.0%
.rspeedy/main/background.*.js 1.17kB 40.22kB 3.0%

@codspeed-hq
Copy link

codspeed-hq bot commented May 7, 2025

CodSpeed Performance Report

Merging #707 will not alter performance

Comparing colinaaa:colin/0507/run-with-force (8a85ec4) with main (aba30cb)

Summary

✅ 1 untouched benchmarks

@colinaaa colinaaa requested review from Yradex and upupming May 9, 2025 08:23
@colinaaa colinaaa marked this pull request as ready for review May 9, 2025 08:23
@colinaaa colinaaa added this pull request to the merge queue May 9, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 9, 2025
@colinaaa colinaaa added this pull request to the merge queue May 9, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 9, 2025
@colinaaa colinaaa added this pull request to the merge queue May 9, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 9, 2025
@colinaaa colinaaa added this pull request to the merge queue May 9, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 9, 2025
@colinaaa colinaaa added this pull request to the merge queue May 9, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 9, 2025
@colinaaa colinaaa added this pull request to the merge queue May 9, 2025
Merged via the queue into lynx-family:main with commit cffbc0e May 9, 2025
33 of 34 checks passed
@colinaaa colinaaa deleted the colin/0507/run-with-force branch May 9, 2025 12:20
@github-project-automation github-project-automation bot moved this to Done in ReactLynx May 9, 2025
colinaaa pushed a commit that referenced this pull request May 9, 2025
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/react@0.108.0

### Minor Changes

- Reverts #239: "batch multiple patches for main thread communication"
([#649](#649))

This reverts the change that batched updates sent to the main thread in
a single render pass.

### Patch Changes

- Add support for batch rendering in `<list>` with async resolution of
sub-tree properties and element trees.
([#624](#624))

    Use the `experimental-batch-render-strategy` attribute of `<list>`:

    ```tsx
    <list
      /**
       * Batch render strategy:
       * 0: (Default) Disabled - No batch rendering
       * 1: Basic - Only batch rendering enabled
* 2: Property Resolution - Batch render with async property resolution
for list item subtree
* 3: Full Resolution - Batch render with async property and element tree
resolution for list item subtree
       */
      experimental-batch-render-strategy={3}
    ></list>
    ```

- rename @lynx-js/test-environment to @lynx-js/testing-environment
([#704](#704))

- Auto import `@lynx-js/react/experimental/lazy/import` when using
`import(url)`
([#667](#667))

- Auto import `@lynx-js/react/experimental/lazy/import` when using
`<component is={url} />`
([#666](#666))

- Fixed a race condition when updating states and GlobalProps
simultaneously.
([#707](#707))

This fix prevents the "Attempt to render more than one `<page />`" error
from occurring during normal application usage.

- Fix error like `Unterminated string constant` when using multi-line
JSX StringLiteral.
([#654](#654))

## @lynx-js/testing-environment@0.1.0

### Minor Changes

- Switch to ESM package format by setting `"type": "module"`.
([#703](#703))

### Patch Changes

- rename @lynx-js/test-environment to @lynx-js/testing-environment
([#704](#704))

## @lynx-js/web-platform-rsbuild-plugin@0.1.0

### Minor Changes

- feat: add new parameter `nativeModulesPath` to
`pluginWebPlatform({})`.
([#668](#668))

After this commit, you can use `nativeModulesPath` to package custom
nativeModules directly into the worker, and no longer need to pass
`nativeModulesMap` to lynx-view.

    Here is an example:

    -   `native-modules.ts`:

    ```ts
    // index.native-modules.ts
    export default {
      CustomModule: function (NativeModules, NativeModulesCall) {
        return {
          async getColor(data, callback) {
            const color = await NativeModulesCall("getColor", data);
            callback(color);
          },
        };
      },
    };
    ```

    -   plugin config:

    ```ts
    // rsbuild.config.ts
import { pluginWebPlatform } from
"@lynx-js/web-platform-rsbuild-plugin";
    import { defineConfig } from "@rsbuild/core";

    export default defineConfig({
      plugins: [
        pluginWebPlatform({
          // replace with your actual native-modules file path
nativeModulesPath: path.resolve(__dirname, "./index.native-modules.ts"),
        }),
      ],
    });
    ```

- feat: Provides Rsbuild plugin for Web projects in Lynx Web Platform,
currently supports polyfill about lynx.
([#606](#606))

## @lynx-js/rspeedy@0.9.4

### Patch Changes

- Bump Rsbuild v1.3.17 with Rspack v1.3.9.
([#708](#708))

- Support `performance.profile`.
([#691](#691))

- Support CLI flag `--mode` to specify the build mode.
([#723](#723))

- Enable native Rsdoctor plugin by default.
([#688](#688))

Set `tools.rsdoctor.experiments.enableNativePlugin` to `false` to use
the old JS plugin.

    ```js
    import { defineConfig } from "@lynx-js/rspeedy";

    export default defineConfig({
      tools: {
        rsdoctor: {
          experiments: {
            enableNativePlugin: false,
          },
        },
      },
    });
    ```

See [Rsdoctor -
1.0](https://rsdoctor.dev/blog/release/release-note-1_0#-faster-analysis)
for more details.

- Bump Rsbuild v1.3.14 with Rspack v1.3.8.
([#630](#630))

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

### Patch Changes

- Fix runtime error: "SyntaxError: Identifier 'i' has already been
declared". ([#651](#651))

- Enable runtime profiling when `performance.profile` is set to true.
([#722](#722))

- fix: resolve page crash on development mode when enabling
`experimental_isLazyBundle: true`
([#653](#653))

- Support `@lynx-js/react` v0.108.0.
([#649](#649))

- Updated dependencies
\[[`ea4da1a`](ea4da1a),
[`ca15dda`](ca15dda),
[`f8d369d`](f8d369d),
[`ea4da1a`](ea4da1a)]:
    -   @lynx-js/react-webpack-plugin@0.6.13
    -   @lynx-js/runtime-wrapper-webpack-plugin@0.0.10
    -   @lynx-js/react-alias-rsbuild-plugin@0.9.9
    -   @lynx-js/react-refresh-webpack-plugin@0.3.2

## @lynx-js/tailwind-preset@0.0.3

### Patch Changes

- Support `hidden`, `no-underline` and `line-through` utilities.
([#745](#745))

## @lynx-js/offscreen-document@0.0.2

### Patch Changes

- feat: support touch events
([#641](#641))

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

### Patch Changes

- feat: support touch events for MTS
([#641](#641))

    now we support

    -   main-thread:bindtouchstart
    -   main-thread:bindtouchend
    -   main-thread:bindtouchmove
    -   main-thread:bindtouchcancel

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

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

### Patch Changes

- fix: some inline style properties cause crash
([#647](#647))

    add support for the following css properties

    -   mask
    -   mask-repeat
    -   mask-position
    -   mask-clip
    -   mask-origin
    -   mask-size
    -   gap
    -   column-gap
    -   row-gap
    -   image-rendering
    -   hyphens
    -   offset-path
    -   offset-distance

- feat: support touch events for MTS
([#641](#641))

    now we support

    -   main-thread:bindtouchstart
    -   main-thread:bindtouchend
    -   main-thread:bindtouchmove
    -   main-thread:bindtouchcancel

- feat: add SystemInfo.screenWidth and SystemInfo.screenHeight
([#641](#641))

- Updated dependencies
\[[`c9ccad6`](c9ccad6),
[`9ad394e`](9ad394e),
[`f4cfb70`](f4cfb70),
[`c9ccad6`](c9ccad6),
[`839d61c`](839d61c)]:
    -   @lynx-js/offscreen-document@0.0.2
    -   @lynx-js/web-mainthread-apis@0.13.1
    -   @lynx-js/web-worker-runtime@0.13.1
    -   @lynx-js/web-constants@0.13.1
    -   @lynx-js/web-worker-rpc@0.13.1

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

### Patch Changes

- fix(web): x-swiper-item threshold updated to 20
([#639](#639))

- fix: In React19, setter and getter functions are treated as
properties, making it impossible to retrieve the current value via
attributes. ([#639](#639))

## @lynx-js/web-explorer@0.0.7

### Patch Changes

- feat: use nativeModulesPath instead of nativeModulesMap to lynx-view.
([#668](#668))

- fix: fork @vant/touch-emulator and make it work with shadowroot
([#662](#662))

- fix: loading errors caused by script import order
([#665](#665))

- chore: update homepage
([#645](#645))

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

### Patch Changes

- fix: some inline style properties cause crash
([#647](#647))

    add support for the following css properties

    -   mask
    -   mask-repeat
    -   mask-position
    -   mask-clip
    -   mask-origin
    -   mask-size
    -   gap
    -   column-gap
    -   row-gap
    -   image-rendering
    -   hyphens
    -   offset-path
    -   offset-distance

- feat: support touch events for MTS
([#641](#641))

    now we support

    -   main-thread:bindtouchstart
    -   main-thread:bindtouchend
    -   main-thread:bindtouchmove
    -   main-thread:bindtouchcancel

- Updated dependencies
\[[`c9ccad6`](c9ccad6)]:
    -   @lynx-js/web-constants@0.13.1

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

### Patch Changes

- feat: support for using `lynx.queueMicrotask`.
([#702](#702))

- feat: support touch events for MTS
([#641](#641))

    now we support

    -   main-thread:bindtouchstart
    -   main-thread:bindtouchend
    -   main-thread:bindtouchmove
    -   main-thread:bindtouchcancel

- feat: provide comments for `@lynx-js/web-platform-rsbuild-plugin`.
([#668](#668))

- Updated dependencies
\[[`c9ccad6`](c9ccad6),
[`9ad394e`](9ad394e),
[`c9ccad6`](c9ccad6)]:
    -   @lynx-js/offscreen-document@0.0.2
    -   @lynx-js/web-mainthread-apis@0.13.1
    -   @lynx-js/web-constants@0.13.1
    -   @lynx-js/web-worker-rpc@0.13.1

## @lynx-js/react-webpack-plugin@0.6.13

### Patch Changes

- feat: add `experimental_isLazyBundle` option, it will disable snapshot
HMR for standalone lazy bundle
([#653](#653))

- Add the `profile` option to control whether `__PROFILE__` is enabled.
([#722](#722))

- Support `@lynx-js/react` v0.108.0.
([#649](#649))

## @lynx-js/runtime-wrapper-webpack-plugin@0.0.10

### Patch Changes

- feat: add `experimental_isLazyBundle` option, it will disable
lynxChunkEntries for standalone lazy bundle
([#653](#653))

## create-rspeedy@0.9.4



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



## upgrade-rspeedy@0.9.4



## @lynx-js/web-worker-rpc@0.13.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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants