algolia / vue-instantsearch

๐Ÿ‘€ Algolia components for building search UIs with Vue.js
https://www.algolia.com/doc/guides/building-search-ui/what-is-instantsearch/vue
MIT License
853 stars 157 forks source link

Vue Router example sandbox does not work #841

Closed jackwkinsey closed 1 year ago

jackwkinsey commented 4 years ago

Bug ๐Ÿž

What is the current behavior?

The code sandbox example that accompanies Combining with Vue Router is broken. The console displays errors stating:

TypeError: Cannot read property 'query' of undefined

This appears to stem from the stateMapping.stateToRoute method in Home.vue.

This seems to cause the products and URL to not update when a refinement filter is selected.

Make a sandbox with the current behavior

Sandbox: https://codesandbox.io/s/github/algolia/doc-code-samples/tree/master/Vue+InstantSearch/routing-vue-router

What is the expected behavior?

When I select a refinement, I should see the product grid update and see the query string added/updated in the URL.

What is the proposed solution?

It looks like this is using old versions of many dependencies (including 3.0.3 of vue-instantsearch). I think this example sandbox just needs updated and the routing object needs to be fixed so the refinement selections work as expected. It does seem like the routeToState method is expecting properties to be there that are not (e.g. query, even when I manually add a query string to the URL).

What is the version you are using?

This sandbox uses these dependencies:

Updating these to their latest versions appear to have no affect. It seems like something is wrong with the actual code.

jackwkinsey commented 4 years ago

I've updated the sandbox as well as the packages within in a forked sandbox.

To get the example to work I had to change instances of instant_search to demo_ecommerce in the routing object in Home.vue.

This is because the name of the index is demo_ecommerce and this is what stateToRoute is looking for.

The selections now work, but using the browser forward and back buttons update the route url but not the state of the app (i.e. hitting back just clears all selections even though there are still some in the url) so it seems like something in routeToState isn't working. Logging out what routeToState is called with shows the following when I click the back button:

routeToState called with:
{
  key: "248216.810"
}

Not sure what that key is but that would explain why the state isn't updated properly as all the properties are found to be undefined so the state is cleared.

Haroenv commented 4 years ago

Looking into this, and it indeed needs more research on how to implement a router with vue router, considering that the back button is overridden by Vue Router. You can listen to the changes with vueRouter.beforeEach (in onUpdate), but the problem is that there's no way to distinguish from a url from a click, and one from a back button.

In the mean time, I'd advise using the history router

jackwkinsey commented 4 years ago

@Haroenv to clarify, when you say "history router" are you referring to the history router from instantsearch.js/es/lib/routers?

Haroenv commented 4 years ago

yes @jackwkinsey, sorry for the confusion here. We will fix this example in the future however

jackwkinsey commented 4 years ago

Does the history router work with server-side rendering? I'm getting an error server side that says window is not defined, which makes sense since it's...you know...a server ๐Ÿ˜„

I assume this is due to the windowTitle function that is setting the title of the window object. I think we'll have to figure out a way to make the vue-router way work. I know we're mostly having issues with the history stack piling up and using the forward and back buttons.

I'll do some more digging and if I find anything that helps I'll share here.

Haroenv commented 4 years ago

ah yes, that's right sorry! What I meant to say is compare the code of the history router and the custom Vue router, and use a code which is much closer to history (using window if available, use vue router just for getting the current URL / reading on server)

calebanthony commented 3 years ago

I've been digging into this for a couple days now. We have an app that's using vue-router and I've been trying to implement a custom router instance.

Updating my _onPopState to this in my onUpdate method shows that my routeToState method is getting the correct parameters. I update it this way because when I hit "back" after navigating in the app, the state is ALWAYS empty, minus the key mentioned earlier. Removing it lets me read from the URL properly.

this._onPopState = ({ state }) => {
  const { key, ...routeState } = state;

  if (Object.keys(routeState).length === 0) {
    cb(this.read());
  } else {
    cb(routeState);
  }
};

However, when I log out what stateToRoute gives me, it doesn't match up:

indexname: {
  configure: {
    distinct: true,
    hitsPerPage: 100,
  }
}
Haroenv commented 3 years ago

I think Vue Router might be using the "state" for their own purposes, and you could fix this by always doing cb(this.read()). Do you have your code in a sandbox where we can inspect @calebanthony ?

calebanthony commented 3 years ago

That's what my edit does in my previous post. This sandbox fixes the problem: https://codesandbox.io/s/dreamy-tdd-3zzqn

And in creating my minimum reproducible example, I found my bug has to lie elsewhere in my application, because that example works.