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

Reader data store: simplify the useIsLoggedIn hook #98558

Merged
merged 1 commit into from
Jan 20, 2025

Conversation

jsnajdr
Copy link
Member

@jsnajdr jsnajdr commented Jan 17, 2025

Simplifies the useIsLoggedIn hook to do one useSelect instead of two.

Also fixes the following bug:

  1. Go to /read/subscriptions
  2. Look at the browser console. You'll see a "Maximum update depth exceeded" React warning. There is some infinite update loop going on.

Try it with this PR. The warning disappears. There is still something weird going on: the useIsLoggedIn hook is called 3000 times during the initial render of the /read/subscriptions route. This PR is an incremental step forward.

@jsnajdr jsnajdr requested a review from tyxla January 17, 2025 19:26
@jsnajdr jsnajdr self-assigned this Jan 17, 2025
@matticbot matticbot added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jan 17, 2025
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

Great catch, @jsnajdr 👍

@jsnajdr jsnajdr merged commit 7727794 into trunk Jan 20, 2025
16 of 17 checks passed
@jsnajdr jsnajdr deleted the update/use-is-logged-in branch January 20, 2025 10:45
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 20, 2025
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