mefechoel / svelte-navigator

Simple, accessible routing for Svelte
Other
504 stars 39 forks source link

Variable assignment redeclares global property #4

Closed hidde closed 4 years ago

hidde commented 4 years ago

In the docs, this line of code:

const location = useLocation(); 

seems to redeclare the global location (window.location). Maybe this should use a different name that doesn't already exist in the browser?

mefechoel commented 4 years ago

It actually does not redeclare it, it shadows the global location. It might be an issue when using var, but Svelte probably wraps the component in a function anyways, so it would still just shadow location.

This is just fine for js engines, and since you should not need to access window.location, when using a Router, shadowing it won't restrict you in development.

You're probably worried about it being confusing to users, rather than browser compatibility though, but I'd still argue, that when using a Router you should rely on it to be your source for location, rather than the browser.

It is also how react-router and @reach/router do it.

I hope that clarifies it :)

hidde commented 4 years ago

Thanks for the explanation, that's very helpful! Was worried about user confusion and compat, but seems neither are warranted here, thanks!