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

fix(responsive): fix shallow equality check on default array value for ParentSize #1370

Merged
merged 4 commits into from
Nov 11, 2021

Conversation

dylanmoz
Copy link
Contributor

@dylanmoz dylanmoz commented Nov 9, 2021

💥 Breaking Changes

🚀 Enhancements

📝 Documentation

🐛 Bug Fix

I noticed that debounceTime wasn't being respected in ParentSize; the children function was being called many times a second. It ended up being because of the default value for ignoreDimensions triggering useMemo to reinstantiate the resize function, which makes the useEffect execute.

I also noticed that children always get's called initially with width 0. I changed that so a layout does not render until an initial width has been measured. We can remove this part of the PR if need be, but it will require users to add a width === 0 ? null : renderContent() type check anytime they use ParentSize to avoid rendering with that "incorrect" initial width.

🏠 Internal

@dylanmoz
Copy link
Contributor Author

@williaster is it possible for me to see the Happo changes? If not just let me know what this PR is changing

@williaster
Copy link
Collaborator

williaster commented Nov 10, 2021

Hey @dylanmoz thanks for digging into the debounce 🙌 hook triggers can be tricky, I think the fix for pulling out the array as a constant makes sense 👍

We can't open up the happo diffs unfortunately but this is what it's showing (left=before, right=after; one is good and one is bad 😅 )
image

I thinkkkk we should probably not make the change to prevent rendering when width === 0 as it would be a breaking change and might explain the xychart happo diff 😞 We do have many checks throughout the demo code/examples currently to make sure width > 0 which reflects the need to do that, but I agree it'd be great not to have to do that. I think it might be simpler to fix the debounce issue here and we could pursue a separate fix for that if needbe.

@dylanmoz
Copy link
Contributor Author

@williaster updated :)

Copy link
Collaborator

@williaster williaster left a comment

Choose a reason for hiding this comment

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

lgtm, thanks again @dylanmoz ! 🙏

@williaster williaster changed the title Fix shallow equality check on default array value for ParentSize fix(responsive): fix shallow equality check on default array value for ParentSize Nov 11, 2021
@williaster williaster merged commit 3a86798 into airbnb:master Nov 11, 2021
@github-actions
Copy link

🎉 This PR is included in version v2.4.1 of the packages modified 🎉

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

Successfully merging this pull request may close these issues.

2 participants