Skip to content
This repository was archived by the owner on Mar 28, 2023. It is now read-only.

navigo router: constructAbsoluteUrl() ignores "useHash" option #440

Closed
jpommerening opened this issue Mar 28, 2017 · 6 comments
Closed

navigo router: constructAbsoluteUrl() ignores "useHash" option #440

jpommerening opened this issue Mar 28, 2017 · 6 comments
Assignees
Milestone

Comments

@jpommerening
Copy link
Member

jpommerening commented Mar 28, 2017

Yo!

navigo.link() does not prepend a hash. With useHash: true and the default hash-symbol, the following function returns things like http://localhost:8500/debug.html/my-place:

function constructAbsoluteUrl( patterns, parameters ) {
   return router.link( constructPath( patterns, parameters ) );
}

We should probably replace it with

function constructAbsoluteUrl( patterns, parameters ) {
   const hashPrefix = useHash ? hash : '';
   return router.link( hashPrefix + constructPath( patterns, parameters ) );
}

fyi @alex3683

@x1B
Copy link
Member

x1B commented Mar 28, 2017

Pretty weird that navigo.link does not use add the hash in the first place!

@jpommerening
Copy link
Member Author

jpommerening commented Mar 28, 2017

Yup. Another option would be to use navigo.generate in constructPath but that would have taken more than 5 minutes. 😅
Also, I'm not sure if constructPath is supposed to prefix it's result with a hash.

@alex3683
Copy link
Member

alex3683 commented Mar 29, 2017

I used to use navigo.generate, but it prepended a hash, that would lead to an error when using the resulting path with navigate, since navigate added another hash. Additionally we needed our special query parameter handling, that would have needed some coding anyways. Since that code was already available in the pagejs router, I simply copied it to the navigo router 😎
Another thing why ignoring navigo.generate was a good thing to do is this breaking change in a bugfix version introduced four days ago: krasimir/navigo#87 🤢 (fun fact: krasimir/navigo#71).

Seems that while switching from generate to the pagejs router code I missed the missing hash. As @x1B already pointed out, the APIs of link and navigate are totally inconsistent regarding the handling of their respective path argument.

I don't like NIH stuff, but it seems finding a basic routing lib that gets out of your way and works is pretty hard ...

Will fix this soonish.

@alex3683
Copy link
Member

Since they don't seem to take semver that seriously, I'll fix navigo to version 4.3.6.

@alex3683
Copy link
Member

Fixed on master (v2.0.0).

@jpommerening
Copy link
Member Author

jpommerening commented Mar 29, 2017

Thanks! 🙏
(For the fix and especially for the thorough explanation what was going on!)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants