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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

URL routing is not working while a custom onStateChange function is defined #4795

Closed
JJWesterkamp opened this issue Jun 25, 2021 · 2 comments 路 Fixed by #4796
Closed

URL routing is not working while a custom onStateChange function is defined #4795

JJWesterkamp opened this issue Jun 25, 2021 · 2 comments 路 Fixed by #4796

Comments

@JJWesterkamp
Copy link

JJWesterkamp commented Jun 25, 2021

馃悰 Bug description

When using the custom onStateChange() implementation together with URL routing the router is not called upon UI state changes.

馃攳 Bug reproduction

Steps to reproduce the behavior:

  1. Enable basic URL routing
  2. Observe the URL being updated upon state changes
  3. Now add a custom onStateChange() function
  4. Observe the URL no longer being updated upon state changes

Live reproduction

Without custom onStateChange() everything works as expected:

https://codesandbox.io/s/inspiring-carson-tc2sv?file=/src/app.js:215-291

Enable the custom onStateChange() and the router stops working:

https://codesandbox.io/s/inspiring-carson-tc2sv?file=/src/app.js

馃挱 Expected behavior

The router is expected to work identically, based on the router configuration, when a custom onStateChange function is either defined or not.

Environment

  • OS: macOS
  • Browser: Chrome
  • Version: 4.24.0-experimental-typescript.0
@tkrugg
Copy link
Contributor

tkrugg commented Jun 25, 2021

Hi @JJWesterkamp thanks for reaching out, and providing an example.
Thats looks like a regression.

@algolia/instantsearch-for-websites we should file bug for this one.
I've reproduced it in this integration test https://github.com/algolia/instantsearch.js/compare/chore/regression-test-onstatechange-middleware
As a workaround for @JJWesterkamp, can we suggest using one of the following?

onStateChange({ uiState, setUiState}) {
    setUiState(nextUiState)

    // add this
    this.middleware.forEach(({ instance }) => {
      instance.onStateChange({
        uiState: nextUiState,
      });
    });
}

@JJWesterkamp
Copy link
Author

@tkrugg Thanks for your reply. The workaround does the trick for now, thanks again!

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 a pull request may close this issue.

2 participants