close
The Wayback Machine - https://web.archive.org/web/20210513004602/https://github.com/facebook/react/pull/21498
Skip to content
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.

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

[Bugfix] Prevent already-committed setState callback from firing again during a rebase #21498

Merged
merged 2 commits into from May 12, 2021

Conversation

@acdlite
Copy link
Member

@acdlite acdlite commented May 12, 2021

First commit adds a failing test.

Second commit fixes it.

acdlite added 2 commits May 12, 2021
Happens during a rebase (low priority update followed by high priority
update). The high priority callback gets fired twice.
Before enqueueing the effect, adds a guard to check if the update was
already committed.
@sizebot
Copy link

@sizebot sizebot commented May 12, 2021

Comparing: b770f75...a807e67

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 126.10 kB 126.12 kB +0.01% 40.46 kB 40.46 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 128.92 kB 128.94 kB +0.01% 41.39 kB 41.40 kB
facebook-www/ReactDOM-prod.classic.js = 406.36 kB 406.40 kB = 75.18 kB 75.19 kB
facebook-www/ReactDOM-prod.modern.js = 394.42 kB 394.46 kB = 73.28 kB 73.29 kB
facebook-www/ReactDOMForked-prod.classic.js = 406.36 kB 406.40 kB = 75.19 kB 75.19 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against a807e67

expect(Scheduler).toHaveYielded([0]);

await ReactNoop.act(async () => {
app.setState({step: 1}, () =>

This comment has been minimized.

@rickhanlonii

rickhanlonii May 12, 2021
Member

Should this be wrapped in startTransition for the default sync case?

This comment has been minimized.

@acdlite

acdlite May 12, 2021
Author Member

For this test it only matters that it's lower priority than discrete/sync (default sync !== discrete sync)

@acdlite acdlite merged commit 46491dc into facebook:master May 12, 2021
34 checks passed
34 checks passed
@facebook-github-tools
Facebook CLA Check Contributor License Agreement is valid!
Details
Image
ci/circleci: RELEASE_CHANNEL_stable_yarn_build Your tests passed on CircleCI!
Details
Image
ci/circleci: RELEASE_CHANNEL_stable_yarn_test_dom_fixtures Your tests passed on CircleCI!
Details
Image
ci/circleci: build_devtools_and_process_artifacts Your tests passed on CircleCI!
Details
Image
ci/circleci: build_devtools_scheduling_profiler Your tests passed on CircleCI!
Details
Image
ci/circleci: get_base_build Your tests passed on CircleCI!
Details
Image
ci/circleci: process_artifacts_combined Your tests passed on CircleCI!
Details
Image
ci/circleci: setup Your tests passed on CircleCI!
Details
Image
ci/circleci: sizebot Your tests passed on CircleCI!
Details
Image
ci/circleci: sync_reconciler_forks Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_build Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_build_combined Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_flow Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_lint Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_lint_build Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test--r=experimental --env=development Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test--r=experimental --env=production Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test--r=stable --env=development Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test--r=stable --env=development --persistent Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test--r=stable --env=production Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test--r=www-classic --env=development --variant=false Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test--r=www-classic --env=development --variant=true Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test--r=www-classic --env=production --variant=false Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test--r=www-classic --env=production --variant=true Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test--r=www-modern --env=development --variant=false Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test--r=www-modern --env=development --variant=true Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test--r=www-modern --env=production --variant=false Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test--r=www-modern --env=production --variant=true Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test_build---project=devtools -r=experimental Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test_build--r=experimental --env=development Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test_build--r=experimental --env=production Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test_build--r=stable --env=development Your tests passed on CircleCI!
Details
Image
ci/circleci: yarn_test_build--r=stable --env=production Your tests passed on CircleCI!
Details
@codesandbox
ci/codesandbox Building packages succeeded.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants