Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Shouldn't this need hooks? #339

Closed
prateekbh opened this issue Oct 3, 2019 · 7 comments · Fixed by #370
Closed

Shouldn't this need hooks? #339

prateekbh opened this issue Oct 3, 2019 · 7 comments · Fixed by #370

Comments

@prateekbh
Copy link
Member

Should we add hooks in this?

some hooks like useRouter would make sense.

const [location, setLocation] = useRouter();

This would allow access of current location on all components and mimic current route functionality by setLocation.

@marvinhagemeister
Copy link
Member

I really like the API wouter has chosen. They do ship with Preact support too 👍

@developit
Copy link
Member

Yup. I'm not sure it makes sense to completely change the API of this module when there's an existing implementation we like.

@gtsop
Copy link

gtsop commented Mar 27, 2020

So what is the suggestion here? We use both preact-router and wouter or just wouter?

@marvinhagemeister
Copy link
Member

@gtsop either router is a great choice 👍 Just don't try to use them at the same time, that always leads to problems regardless of the framework you're using.

@developit
Copy link
Member

developit commented May 9, 2020

I think it might be worth exporting hooks from preact-router/match. It's already an add-on lib, and the subscription mechanism is already there to be able to implement reactivity. It likely wouldn't even need to use the new context API.

@toniopelo
Copy link
Contributor

I think it might be worth exporting hooks from preact-router/match. It's already an add-on lib, and the subscription mechanism is already there to be able to implement reactivity. It likely wouldn't even need to use the new context API.

That would be great @developit
I'm new to preact and maybe I'm not enough familiar with it yet. But I feel like this simple use case is not covered:

<Component class={currentPath === '/some/path' ? 'active' : ''}>
    ...
</Component>

In some situation Component cannot be changed with Link because it's not a link and <Match></Match> can't do the job without duplicating code (one Component with 'active' and another Component without 'active'). And probably even causing component recreation when my snippet wouldn't ? (not sure on this one)

Anyway it feels like having access to the match internals through the use of a hook would be great 👍
Would be happy to submit a PR if I find the time for it ⏳ 🙈

@toniopelo
Copy link
Contributor

toniopelo commented Jun 12, 2020

I think it might be worth exporting hooks from preact-router/match. It's already an add-on lib, and the subscription mechanism is already there to be able to implement reactivity. It likely wouldn't even need to use the new context API.

Created a PR for this but I finally decided to use the Context API. The reason is to be able to keep this hook working when your app uses several Router instances. With the Context API I can ensure that I get the values corresponding to the router the component is depending on.

The PR: #370

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

Successfully merging a pull request may close this issue.

5 participants