rundfunk47 / stinsen

Coordinators in SwiftUI. Simple, powerful and elegant.
MIT License
907 stars 95 forks source link

add RouterStore and RouterObject PropertyWrapper #10

Closed savage7 closed 3 years ago

savage7 commented 3 years ago

This is a follow up to https://github.com/rundfunk47/stinsen/issues/5

As I described in the issue I use Resolver for injecting my dependencies. Since using it directly within Resolver is messy I created a RouterStore which kind of copies the SwiftUI Environment behavior. It allows to use the Router without directly passing them from via the View, so there is no tight coupling.

I created a new protocol called RouterIdentifiable which has to be implemented optionally by a Coordinator. This way the code is 100% backward compatibly.

It would be cool if you like my solution our if we could find a similar solution enabling better ViewModel usage :blush:

rundfunk47 commented 3 years ago

Hi! This looks like the beginning of a good solution! 👍🏽 I'll give it a good look later this week :)

savage7 commented 3 years ago

It would also possible to create it without id just with type like this, without id. In this case the id could be the type description:

 @RouterObject
  var unauthenticated: NavigationRouter<UnauthenticatedCoordinator.Route>?

I'm not sure how we can handle possible duplicates then. The Swiftui Environment has a tree structure based on the view render tree. The RouterStore is just a plain Dictionary.

rundfunk47 commented 3 years ago

Hi! I haven't had the time to really give this a through look, holidays and all. But there were one thing I noticed:

The RouterStore should probably be a Dictionary<String, Array<...>>. Right now the Testbed does not work when pushing two of the "same" coordinator. The retreive-function should just pick the latest created router, that ought to be a good enough solution when viewmodels are created at the same time as the view.

I'll give it a closer look ASAP though! :)

rundfunk47 commented 3 years ago

I implemented the changes I suggested now @savage7 , it seems to work with the Testbed now :) . Does these changes look good?

savage7 commented 3 years ago

@rundfunk47 I'm on vacation too so I couldn't compile it :sunny:. But from looking at the code your solution looks cleaner, thanks for looking into it :blush:

rundfunk47 commented 3 years ago

Great! I'll just clean it up a bit and then I'll merge it 😄