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

Avoid unwanted re-render on resize #1705

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dubzzz
Copy link

@dubzzz dubzzz commented Apr 13, 2022

While digging into the source code of the library to investigate a runtime crash I had in one of my app, I found this quick optimization. I though it could still be useful to share it but feel free to reject it if it does not make sense.


Up-to-now the current implementation watches the event onWindowResize and triggers a state update anytime it receives an event related to resize even if the size of the offset was not updated.

The PR just removes this unneeded re-render by not updating the state in such case.

Up-to-now the current implementation watches the event `onWindowResize` and triggers a state update anytime it receives an event related to resize even if the size of the offset was not updated.

The PR just removes this unneeded re-render by not updating the state in such case.
@github-actions github-actions bot added the core use this label for changes in `lib` directory label Apr 13, 2022
@github-actions
Copy link

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this PR will be closed in 7 days

@github-actions github-actions bot added the stale The label to apply when a pull request or an issue is stale label Jul 12, 2022
@STRML
Copy link
Collaborator

STRML commented Jul 12, 2022

Thanks. It seems fairly unlikely that the window would resize and the grid wouldn't change width, unless of course it was being used in a fixed-width layout. But if it were, why would you be using <WidthProvider> at all?

@dubzzz
Copy link
Author

dubzzz commented Jul 12, 2022

Trying to find back why I needed it 😂 Will come back to you when I get a better idea of my original need (don't remember at all why I proposed the fix)

I think I initially opened the PR because doing the quick ternary can save some unwanted re-render I got on my production app from time to time because of re-renders on parent components... As the re-render was quick to avoid I suggested this change as it removes one unneeded render even if such render is rare as it can still happen from time to time.

Mostly a free optimization 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core use this label for changes in `lib` directory stale The label to apply when a pull request or an issue is stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants