Possibly misleading documentation for "Combining with Vue Router"

[nobitagit]

Bug 🐞

Hello. This is not really a bug, but a possible mistake in the docs. Let me know if I am missing something.

What is the current behavior?

If you check the docs for the routing > Combining with Vue Router these lines appear in the example:

      this._onPopState = event => {
        const routeState = event.state;
        // On initial load, the state is read from the URL without
        // update. Therefore, the state object isn't present. In this
        // case, we fallback and read the URL.
        if (!routeState) {
          cb(this.read());
        } else {
          cb(routeState);
        }
      };
      window.addEventListener('popstate', this._onPopState);

It looks to me these lines are not needed and actually break the browser back/forward button.

Make a sandbox with the current behavior

Take this sandbox.

• Click on a few of the filters
• The view should update accordingly (good)
• Click "back" once
• The view loses the filters entirely (bad)
• Click "forward" once
• The view doesn't render the correct filters (bad)
• Refresh. The view should appear with correct filters (good)

What is the expected behavior?

Take this other sandbox where I commented out the lines I posted at the beginning of this issue.

• Click on a few of the filters
• The view should update accordingly (good)
• Click "back" once
• The view updates the filters correctly (good)
• Click "forward" once
• The view updates the filters correctly (good)
• Refresh. The view should appear with correct filters (good)

Does this happen only in specific situations?

.

What is the proposed solution?

Delete the lines from the docs, if they are not needed. Otherwise I'd be curious to know which corner case these lines are there for, so I avoid introducing bugs in my own app.

Many thanks, have a nice weekend.

What is the version you are using?

Always try the latest one before opening an issue.

[Haroenv]

It's indeed complicated to keep the state correctly in sync, and depending on your use case, that code will not be needed, but it's required for the flow:

1. do a refinement (via InstantSearch or via a vue router url)
2. click on a regular url (like in the header)
3. state should update to previously set InstantSearch state, but isn't

That is because pop state is triggered for "regular" navigations, ones that don't use pushState

It's indeed true the example probably could be simplified though, although it's a very fragile part of the platform IMO

[Haroenv]

Thanks again for this concern. The main point is not reading from history.state. That's indeed correct and the change was made in InstantSearch.js in https://github.com/algolia/instantsearch.js/pull/5177. I also made a PR to the documentation that will close this issue when merged to transform those snippets to always read from the URL without trusting history.state