Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

8.7.1 Using a hash is ignored when not using a hash based routing #269

Closed
ajorkowski opened this issue Jan 23, 2021 · 13 comments
Closed

8.7.1 Using a hash is ignored when not using a hash based routing #269

ajorkowski opened this issue Jan 23, 2021 · 13 comments

Comments

@ajorkowski
Copy link

I do not use the hash based routing, but it looks like the path gets ignored when creating a match.

Here is an example to illustrate, in my app I navigate to the path:

/appointments?#start=2021-01-01&appointmentId=1

However the match on the uses has the following structure:

{
    data: null,
    params: null,
    queryString: "",
    route: { ... },
    url: "appointments"
}

There is no information in the match at all about the hash url, when before this was available in the query option that used to be available. Is there any way to get the hash information? It would be ideal if we didn't read the url in the browser ourselves because I think it may not be consistently there (especially because we are using a separate router for legacy paths).

Would it make sense to get the hash part included in the match?

Cheers,
Felix

@krasimir
Copy link
Owner

Hm ... this seems a bit weird to me. I mean the string after the # sign could be accessed via location.hash. And this is supported well on all browsers so should be good enough to use it directly. Navigo ignores the string after # on purpose and if we have to change the API and make that part of the Match object it will be probably just:

{
  hash: location.hash
}

So I guess it is the same as you directly using location.hash.

P.S.
by the way Navigo heavily relies on location.hash. So if that's not working in your context (like node, deno, a browser on a mobile device) then I guess this whole library will not work properly.

@ajorkowski
Copy link
Author

Yeah that makes sense actually - I am using two routers but one is to support legacy as we migrate things, but both are still working off the url, so I think this will work fine. I appreciate your response! I will close this issue.

@ajorkowski
Copy link
Author

Actually, I know what the issue is now that I tried to implement this again. I am using the navigate method to change the url. So you get something like this:

navigo.navigate('/appointments?#start=2021-01-01&appointmentId=1')

Navigo calls the hook before changing the url in the browser, so I cannot use location.hash in this case during the hook, because the url has not been set in the browser yet? Maybe I am using this wrong though....

Cheers,
Felix

@ajorkowski ajorkowski reopened this Jan 25, 2021
@ajorkowski
Copy link
Author

Just to expand on this a little, I am using the navigo.on({ path: { as: name, uses: func } }) to setup the routes, so the func here is the hook method that gets called before the url is set.

@krasimir
Copy link
Owner

I see. Interesting case @ajorkowski. I'll take a step back and will ask what's the main goal. It looks (for me) that you need to do some stuff before to get the route resolved that depend on the hash. Is it possible to solve the problem with a bit of composition. For example now you have:

> navigate to appointments?#start=2021-01-01&appointmentId=1
> work A that needs "start=2021-01-01&appointmentId=1"
> route resolving
> work B

What about

> work A that needs "start=2021-01-01&appointmentId=1"
> navigate to appointments
> route resolving
> work B

or

> navigate to appointments
> route resolving
> work A that needs "start=2021-01-01&appointmentId=1"
> work B

I mean by extracting your tasks in separate functions isn't it possible to fulfill your needs with calling those functions in right order.

@ajorkowski
Copy link
Author

I guess I could rearrange how the functions work... but if I go by your steps I don't understand how I would say do navigate to appointments without using the navigo library? How could I navigate to appointments?#start=2021-01-01&appointmentId=1 and have that in the url without refreshing the page and without using the navigate method, and making sure the history is all correct etc?

If I have some way of setting the url first then it would work, but isn't the point that I can give navigo a path and it is supposed to resolve this using the rules I have and set the url with history kind of thing? This would take away the utility of it I guess?

P.S. This is just an example route, where possibly this route could be changed to query parameters, but there are some parameters which I wouldn't want to get sent to the server if possible (like temporary access tokens #token=...).

@ajorkowski
Copy link
Author

ajorkowski commented Jan 26, 2021

As for what I am trying to do - when I go to any page like appointment/:id or appointments?id=:id etc I have a react component and it just needs to know those parameters in the url/query/hash, so that it can change what information it will show. With the hash I could potentially wait until the component itself and use location.hash, but the problem is that I cannot use the navigate method with a url with a hash, because all that will end up in the actual browser will be the url without the hash, so I cannot access location.hash even in the react component.

@krasimir
Copy link
Owner

I see. Maybe I wasn't clear with my pseudo snippets above. What I meant was rearranging the bits that need to read from the hash so you let Navigo changes the URL and inside the handler you consume the has via location.hash.

In general this one /appointments?#start=2021-01-01&appointmentId=1 seems weird to me. The hash makes sense to be an id of a DOM element so the browser can scroll to it. What stops you to write /appointments?start=2021-01-01&appointmentId=1 (without a hash) and use match.params?

@ajorkowski
Copy link
Author

But that's what I'm trying to get at - There is no way for Navigo to change the url in the browser with a hash in it? If you try and navigate to a url with a hash it will be removed so you end up with a hash-less url in the browser bar. This means I would have to store what would normally become the hash part internally somewhere instead of it being in the url so I can look it up instead of using location.hash? That might actually work, though the problem is that the history (which is set by navigo) won't have the hash, so it kind of breaks there - pressing back etc on the browser would break.

As for your second point. In this specific case there isn't anything stopping us from changing to use params - it's just a weird legacy thing that we are eventually going to replace... Though that isn't now unfortunately because there are live links out there that are using this format.

Saying that, I think there are times when having a hash is valuable instead of using a query string, and that is the fact that hashes do not get sent to the server. So I have something like /viewbill?#token=... would only make a request for /viewbill to the server, reducing the surface area the token is available.

I think having a library that is supposed to handle urls should probably also handle hashes, because they have a different role than query params in the browser in this way (not just a redundant way to represent params in urls), but that's just me. It's just unfortunate that all of a sudden it's not supported because it's what I need at this point - it's all good we will just have to lock the version - I only started this thread because I just thought maybe it was overlooked as opposed to it being a philosophical difference in the lib!

Thanks for your help working through this though!

krasimir pushed a commit that referenced this issue Jan 27, 2021
@krasimir
Copy link
Owner

@ajorkowski can you please try new 8.8.0 version. I added a hashString field into the Match object.

https://codesandbox.io/s/navigo-example-jrui8

import Navigo from "navigo";

document.getElementById(
  "app"
).innerHTML = `<a href="/user/xxx/save?foo=bar#anchor-here" data-navigo>click me</a>`;

const router = new Navigo("/");

router.on("/user/:id/:action", function (match) {
  document.getElementById("app").innerHTML = `<pre>${JSON.stringify(
    match,
    null,
    2
  )}</pre>`;
});

Clicking the link shows the following:

{
  "url": "user/xxx/save",
  "queryString": "foo=bar",
  "hashString": "anchor-here",
  "route": {
    "name": "user/:id/:action",
    "path": "user/:id/:action",
    "hooks": {}
  },
  "data": {
    "id": "xxx",
    "action": "save"
  },
  "params": {
    "foo": "bar"
  }
}

@ajorkowski
Copy link
Author

Awesome, this looks like it will work really well. I will try to use the new 8.8 version and let you know how it goes. Probably won't get to it until next week however.

@krasimir
Copy link
Owner

krasimir commented Feb 1, 2021

Hey @ajorkowski. Can we close this issue?

@ajorkowski
Copy link
Author

@krasimir Yes I can confirm works great, I was able to upgrade navigo with these changes :) I'll close it now.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants