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

Faster Paint with delayed flush of Effects #1435

Conversation

knaveenkumar3576
Copy link
Contributor

@knaveenkumar3576 knaveenkumar3576 commented Jun 12, 2024

This pull request enables faster paint of chat switch the changes to DND kit with the following changes:

Change 1: Changing useIsoMorphicLayoutEffect to useEffect
Reason: Changing state in a layout effect can delay browser paint.

Details: The DND kit mutates state in a layout effect, but only for registering elements as draggable. This hurts performance as changing state in a layout effect can delay browser paint. In this case, there is no need to change state in a layout effect because this state change is only registering draggable elements which can wait until browser paint.

Example: A CodeSandbox example demonstrates how setting state in a layout effect delays browser paint.

Case 1: Changing state in a layout effect.
image
Screenshot showing Paint after flush.

Case 2: Changing state in an effect.
image
Screenshot showing Paint before flush.

Change 2: Updating useRect and useRects
Reason: Removed the reducer implementation in favor of state.

Details: Similar to the first point, the state was changing in a layout effect when a user clicks and drag a DND node. Initially, we considered replacing useIsoMorphicLayoutEffect with useEffect. However, to avoid visual discrepancies, we replaced the useReducer with useState. This change ensures that even though the state updates within a layout effect, the paint is delayed only when there is a meaningful state change. This is because useReducer doesn't bail out of meaningless state updates, whereas useState can.

Automated Tests:
We also ran Cypress and Unit Test on github repo to gain confidence on proposed changes. Here are the results
image

image

Copy link

changeset-bot bot commented Jun 12, 2024

🦋 Changeset detected

Latest commit: 6bbe39b

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

This PR includes changesets to release 1 package
Name Type
@dnd-kit/core 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

@knaveenkumar3576 knaveenkumar3576 marked this pull request as ready for review June 14, 2024 22:57
@knaveenkumar3576 knaveenkumar3576 changed the title replace useReducer with useState Faster Paint with delayed flush of Effects Jul 5, 2024
Copy link
Owner

@clauderic clauderic left a comment

Choose a reason for hiding this comment

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

Thank you!

@clauderic clauderic merged commit 2eb636f into clauderic:master Nov 23, 2024
@github-actions github-actions bot mentioned this pull request Nov 23, 2024
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

2 participants