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(next-app-router): hydration error breaks routing #6542

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

aymeric-giraudet
Copy link
Member

Summary

Fixes #6541

Result

Kind of hacky, but hydration errors seem to trigger effect cleanup functions while not actually remounting the components ?

Note we wouldn't have this problem if we had an InstantSearch instance outside of React lifecycle, which should probably be the way to go for the stable release.

@aymeric-giraudet aymeric-giraudet requested review from a team, dhayab and sarahdayan and removed request for a team January 22, 2025 15:20
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit edd19a9:

Sandbox Source
example-instantsearch-getting-started Configuration
example-react-instantsearch-getting-started Configuration
example-react-instantsearch-next-app-dir-example Configuration
example-react-instantsearch-next-routing-example Configuration
example-vue-instantsearch-getting-started Configuration

Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

no test possible for this case? maybe that we don't try to write history if the component unmounts, or does it just error at that point before this PR?

@aymeric-giraudet
Copy link
Member Author

aymeric-giraudet commented Jan 22, 2025

@Haroenv it would be better with e2e tests as we could properly replicate the hydration issue.

I can add tests maybe but this really is a quickfix intended to be removed once we ensure we always have a single instantsearch instance.

@Haroenv
Copy link
Contributor

Haroenv commented Jan 23, 2025

all good @aymeric-giraudet !

@aymeric-giraudet aymeric-giraudet merged commit e598230 into master Jan 23, 2025
15 checks passed
@aymeric-giraudet aymeric-giraudet deleted the fix/hydration-broken-routing branch January 23, 2025 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Url params not updating when using react-instantsearch-nextjs (after upgrade to 0.4.2)
3 participants