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(react-router): improve memoization for context providers #9983

Merged
merged 7 commits into from
Mar 3, 2023

Conversation

appden
Copy link
Contributor

@appden appden commented Jan 26, 2023

This will reduce some unnecessary re-renders where the context is accessed.

@changeset-bot
Copy link

changeset-bot bot commented Jan 26, 2023

🦋 Changeset detected

Latest commit: 1417fdb

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

This PR includes changesets to release 4 packages
Name Type
react-router Patch
react-router-dom Patch
react-router-dom-v5-compat Patch
react-router-native 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

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Jan 26, 2023

Hi @appden,

Welcome, and thank you for contributing to React Router!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run.

Thanks!

- The Remix team

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Jan 26, 2023

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

This will reduce some unnecessary re-renders where the context is accessed.
@MichaelDeBoey MichaelDeBoey changed the title Improve memoization for context providers fix(react-router): improve memoization for context providers Jan 27, 2023
@brophdawg11
Copy link
Contributor

@appden Thanks for this! Do you happen to have an example of the useBlocker issue you're fixing with the latest commit? I'm tempted to deal with that separately...

@appden
Copy link
Contributor Author

appden commented Feb 2, 2023

@appden Thanks for this! Do you happen to have an example of the useBlocker issue you're fixing with the latest commit? I'm tempted to deal with that separately...

No prob! I made that commit because I originally neglected to run tests (doh) and saw several tests in use-blocker-test.tsx fail on CI that I presume would have blocked merging this in.

@brophdawg11
Copy link
Contributor

Ah ok if they were caused by these changes that makes sense. I'll try to do some deeper testing on this next week 👍

@brophdawg11 brophdawg11 self-assigned this Feb 2, 2023
@appden
Copy link
Contributor Author

appden commented Feb 15, 2023

Hi @brophdawg11, kindly follow up to see if you've had the chance to test this further. Thanks!

@brophdawg11 brophdawg11 changed the base branch from main to dev February 22, 2023 15:24
@brophdawg11 brophdawg11 changed the base branch from dev to release-next February 22, 2023 16:57
@brophdawg11 brophdawg11 changed the base branch from release-next to dev February 22, 2023 16:57
@brophdawg11 brophdawg11 merged commit 0823407 into remix-run:dev Mar 3, 2023
@brophdawg11
Copy link
Contributor

Thanks @appden!

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.

3 participants