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(router): skip router write on duplicate entries #4487

Merged
merged 3 commits into from
Aug 24, 2020

Conversation

francoischalifour
Copy link
Member

@francoischalifour francoischalifour commented Aug 24, 2020

This fixes duplicate entries in the router that happen when router.write is called even though uiState hasn't changed.

Explanation

We used to call the write method of the router at every state change. A state change happens at each UI interaction or whenever a widget is mounted/unmounted. The latter is problematic in component-based flavors of InstantSearch (Vue InstantSearch is the only one relying on InstantSearch 4 right now) because widgets are often added/removed.

The duplicate entry detection check was happening in the history router itself, which meant that whenever users opt-out of this default router and implemented their own, they would get this issue.

This fix checks that router.write needs to be called in the router middleware itself. Since it's one level above the history router, or custom user implementation, it will get fixed no matter the router implementations. We check that the routeState is different from the last received to call router.write. We base the check on routeState, and not uiState, because users might plug their own logic in stateToRoute that returns a routeState that differs from uiState; the two do not necessarily change at the same time.

Related

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 24, 2020

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 068099c:

Sandbox Source
InstantSearch.js Configuration

Copy link
Contributor

@eunjae-lee eunjae-lee left a comment

Choose a reason for hiding this comment

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

Looks good to me except for the little thing in the test.

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.

makes sense :)

@francoischalifour francoischalifour merged commit 9296022 into master Aug 24, 2020
@francoischalifour francoischalifour deleted the fix/router-skip-duplicate-entry branch August 24, 2020 17:52
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.

Navigation history is being flooded with duplicate paths
3 participants