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

synchronous forceUpdate #3928

Closed
wants to merge 1 commit into from
Closed

Conversation

KevinDoughty
Copy link
Contributor

The comment for the function forceUpdate says:
"Immediately perform a synchronous re-render of the component"

This PR restores that behavior. Does not cause any tests to fail. I cannot find much information on a "double render" problem which sounds dubious to me.

Animation libraries need timely updates and there is no reason to arbitrarily deny that. Please use setState({}) instead.

See:
#1939

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

📊 Tachometer Benchmark Results

Summary

A summary of the benchmark results will show here once they finish.

Results

The full results of your benchmarks will show here once they finish.

tachometer-reporter-action v2 for Benchmarks

@KevinDoughty
Copy link
Contributor Author

My mistake, tests fail. Will try again soon.

@marvinhagemeister
Copy link
Member

Seeing that the comment was made 5 years ago, it's very likely that it is out of date. In general you don't want to kick off a render whilst another one is currently being processed. That was the main reason for us going with enqueueRender() for .forceUpdate() if memory serves correctly.

The current scheduling mechanism uses a microtask which is the smallest unit of scheduling in the event loop and it runs <1ms. Did you run in a specific problem where that caused problems with an animation?

@marvinhagemeister
Copy link
Member

Haven't played with the view transitions API yet, but it looks like it might require a synchronous flush of the queue: #3929

@sschultze
Copy link

Haven't played with the view transitions API yet, but it looks like it might require a synchronous flush of the queue: #3929

@marvinhagemeister The flush of the queue actually doesn't have to be synchronous. The callback passed to startViewTransition can return a promise, as described here.

But I think that there's a chance that 3rd party libraries will use React's flushSync with the View Transition API, so it's probably better to implement this rather than introducing another async function.

@KevinDoughty
Copy link
Contributor Author

Solved my problem without synchronous renders. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants