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

withSnackbar HOC re-rendering child unnecessarily #39

Closed
martinmckenna opened this issue Dec 14, 2018 · 3 comments
Closed

withSnackbar HOC re-rendering child unnecessarily #39

martinmckenna opened this issue Dec 14, 2018 · 3 comments
Labels
must have really? it's 2019 and still we don't have this feature?

Comments

@martinmckenna
Copy link
Contributor

martinmckenna commented Dec 14, 2018

Expected Behavior

in withSnackbar.js, there should be some sort of shallow props checking so that it ensures that it is not unnecessarily re-rendering it's child component. I suppose the shallow prop check would just check whether it already has access to the notistack props?

Current Behavior

Because of the lamda functions below, the child component is getting re-rendered with every single update because of the functions are getting reinstantiated, regardless of the child component already having access to the notistack props.

        {handlePresentSnackbar => (
            <SnackbarContextNext.Consumer>
                {handleEnqueueSnackbar => (
                    <Component
                        {...props}
                        onPresentSnackbar={handlePresentSnackbar}
                        enqueueSnackbar={handleEnqueueSnackbar}
                    />
                )}
            </SnackbarContextNext.Consumer>

Steps to Reproduce

Link:

It turns out this the bug that I found in my project wasn't actually caused by this, but I'm going to leave it open because having lamdas in render functions is inherently bad for performance and the above description still stands true.

@iamhosseindhv iamhosseindhv added the must have really? it's 2019 and still we don't have this feature? label Dec 29, 2018
@g-bastianelli
Copy link

I have similar problem. Children rerender when notification disappear.

@iamhosseindhv
Copy link
Owner

iamhosseindhv commented Feb 23, 2019

@g-bastianelli This will be fixed in the next release.

UPDATE:
this couldn't make it to v0.4.3 that I published today. So it'll be published in the next update next week.

@iamhosseindhv
Copy link
Owner

The reason for consumers re-rendering wasn't usage of lambda functions. React used to update consumers when value of SnackbarContext.Provider had changed, and this used to happen on every render because a new object for value prop was getting created.

Keeping reference to value object identical solved the issue.

Thanks everyone for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
must have really? it's 2019 and still we don't have this feature?
Projects
None yet
Development

No branches or pull requests

3 participants