veliovgroup / flow-router

🚦 Carefully extended flow-router for Meteor
https://packosphere.com/ostrio/flow-router-extra
BSD 3-Clause "New" or "Revised" License
201 stars 30 forks source link

Added typescript tests and updated readme for tests #99

Closed timsun28 closed 7 months ago

timsun28 commented 1 year ago

I haven't taken the time to write a lot of tests, but this can work as a starting point for anyone else to add more tests where there is a need for them.

donflopez commented 1 year ago

Hey, thanks for your PR! I'm looking forward for this to be merged. I noticed that Redirect method is also missing from the Router type. Is there any plan to migrate the router to TS vs having types separately?

dr-dimitru commented 1 year ago

Hello @donflopez

Is there any plan to migrate the router to TS

No plans.

We have plans to work on a new router from scratch

@timsun28 @donflopez let me know if you plan to push any other changes/additions into this one or it is good to get merged?

donflopez commented 1 year ago

We have plans to work on a new router from scratch

Awesome! I don't have much time but I'd love to contribute as much as I can if you're open to it. Looking forward to it!

let me know if you plan to push any other changes/additions into this one or it is good to get merged?

It's all @timsun28 decision. I'll push types for redirect method (if he doesn't do it) in a different PR.

PD: I have a new PR open with just a small change to the Readme so people know how to use the types in this package.

timsun28 commented 1 year ago

I would be happy to help with adding types for redirect method. I'm not sure though what you mean with this. I don't use the method anywhere in my app, but I did previously add the redirect in a trigger type as the Router go() function.

Let me know what I can add, and if you prefer to create a pr for this yourself, feel free to do so.

We have plans to work on a new router from scratch

That sounds cool! What are your plans in terms of features? I've been recently started migrating my app from blaze to react and looking for a different router, because I wasn't happy with the React integration with FlowRouter right now. Would be cool to if React was supported properly in a new Router.

And in terms of typescript, I have heard of having typed routes, that would add some real value to the router! Even though I'm quite busy, I would be happy to help if you're in need of anything.

donflopez commented 1 year ago

The one I mean is here:

https://github.com/veliovgroup/flow-router/blob/master/client/router.js#L319

I'm using this locally, haven't dive deep.

type Router = {
...
redirect: (path: string) => void;
...
};
timsun28 commented 1 year ago

Got it, I will add the redirect function to this pr now. I initially didn't add it, because I used the wiki as a guide for the types. I see now that there are other functions missing as well.

track(reactiveMapper)
mapper(props, onData, env)
trackMapper()
subsReady()
withTrailingSlash(fn)

@dr-dimitru Are the following functions (and redirect(path)) not in the wiki because they aren't meant to be used or should we add them to the types and wiki as well?

dr-dimitru commented 1 year ago

Hello @timsun28 @donflopez let me know if it's ready to get merged or you're planning to have more commits?

@dr-dimitru Are the following functions (and redirect(path)) not in the wiki because they aren't meant to be used or should we add them to the types and wiki as well?

this is helper functions meant for internal use. I assume

timsun28 commented 1 year ago

Hi @dr-dimitru, seeing how @donflopez is using these methods, I think we should add them to the types for now (they are included in this pr). I had no other plans for additional commits, so this branch is ready to be merged.

I'm currently running into a different problem with FlowRouter and a project with both Blaze and React pages/components. My current approach to this is creating blaze pages with the {{>React}} helper that creates the actual page, until I can completly switch to another router. Seeing how this solution isn't supported any more with the latest React versions, is there a different way people are implementing React with FlowRouter?

And I had a look at the more popular React Router which works quite well in Meteor, but I was also wondering if you have had the chance to start working on a new router we previously discussed.

dr-dimitru commented 7 months ago

@timsun28 Thanks a lot for your contribution, working on release now. I'm sorry that it took that long

And I had a look at the more popular React Router which works quite well in Meteor, but I was also wondering if you have had the chance to start working on a new router we previously discussed.

Hopefully it will happen in 2024 🤞

timsun28 commented 7 months ago

@dr-dimitru No worries for taking so long, I know how life can get busy..

In the past year, I've moved my main application away from Blaze and went all in on React and React Router.

It seems that the current trend for routers is mostly going towards "file-based routing" like Next.js, remix, Tanstack router (really cool implementation for type safe routing).

I'm currently still using React Router and am happy with the client side routing for my needs.

For Meteor in general, I think it would be valuable to build some very simple sample projects with the different router options and describe their pros and cons, so new/existing users can make an informed decision on what router to go with.