hyperapp-router icon indicating copy to clipboard operation
hyperapp-router copied to clipboard

Router via CDN requires renaming "location"

Open zaceno opened this issue 7 years ago • 6 comments

See this codepen:

https://codepen.io/zaceno/pen/RxRjVZ

It is a simple copy-paste of the first example in the README.md. Only I've changed

import { Link, Route, location } from "@hyperapp/router"

to

const { Link, Route, location } = router

because the router is loaded via CDN.

... and notice that it doesn't work.

The reason is some kind of collision between the router's location and window.location. It can be worked around by renaming location, in the destructuring, as I have done here:

https://codepen.io/zaceno/pen/YYWEQV?editors=0010

This should be noted in the README.md, or perhaps "location" should be called something else.

zaceno avatar Dec 20 '17 20:12 zaceno

I am currently using

import { location as router } from "@hyperapp/router"

In my build too, since I met some troubles.

Swizz avatar Dec 20 '17 21:12 Swizz

I guess we could rename location to router, but there's a reason why it was called location in the first place. I need to think about this one a bit. In the meantime could you use the solution described by @Swizz?

jorgebucaran avatar Dec 21 '17 01:12 jorgebucaran

This has also caused problems for me; I'm not sure if I'm doing something else wrong, but:

router.Link({ to: "/test-url", location: router.location }, "test link")

works, while

router.Link({ to: "/test-url" }, "test link")

gives me t.set is not a function in the "handleLocationChange" function.

I'm unable to get actions.location.go("...") to work at all due to the same exception.

jameswilddev avatar Jan 04 '18 15:01 jameswilddev

I'll have @hyperapp/router 1.0 ready this weekend! 🙏

jorgebucaran avatar Jan 04 '18 15:01 jorgebucaran

@JorgeBucaran In the meantime could you use the solution described by @Swizz?

@Swizz solution is basically the same as the workaround I suggested, but for es6 modules rather than CDN (which is the issue I was reporting on). I haven't had any issues like @Swizz described using the router as a module in a rollup-bundle though.

I'm not sure you need to rename it. I think it would be enough to just make a note in the docs:

"Make sure you don't accidentally overwrite window.location with router.location. It can happen easily if you destructure router in the global scope like so: const {..., location} = router. This will break things!"

zaceno avatar Jan 09 '18 08:01 zaceno

Temporary fix

I am currently using the object destructuring's assignation to new variable names technique to rename all location to something else. This way, I am able to use the location property, but aliased. I am still having some trouble with the Link component, so I have made a custom Go component to go from one or another route.

Live Demo

See my JSFiddle demo to learn more.

aminnairi avatar Jun 20 '18 11:06 aminnairi