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

Departure from ReSwift-Router #22

Open
NemoOudeis opened this issue Jun 7, 2019 · 1 comment
Open

Departure from ReSwift-Router #22

NemoOudeis opened this issue Jun 7, 2019 · 1 comment

Comments

@NemoOudeis
Copy link

Hi,

we are using the router and found a couple of shortcomings (from our perspective).
We decided to develop those missing features in our fork and have cleaned up a lot of implementation details and test cases.

Our fork -> https://github.com/rakutentech/rekotlin-router

I have not raised a pull request for these changes because we move away from the old API contract that came from the ReSwift-Router port, most notably:

  • A route is no longer a list of strings. It is now a List of RouteSegments (a data class, but effectively it's a list)
  • Route segment is essentially a `Pair<String, Any?>
  • Route specific data is gone, instead every route segment has its arguments in the second part of the RouteSegment

We made this change because the setting data & setting route separation proved to be very awkward, e.g. after route change the router would create a new component, show it, then the components needs to subscribe to the store to look up its data - which prevents us from using constructor injection for route arguments.

There's more to the discussion, but it's not that important really - I'm not trying to convince you to adopt this. Rather than that i'm offering our perspective - if you are interested we can merge that back to you guys. If not we will maintain our fork separately.

Even if you are not interested in the functional changes you might be interested in the cleaned up test cases: I made away with power mock and updated mockito, used constructor injection for the handler to pass a fake and simplified the test cases a lot. Anyways, take it or leave it.

If you want to have a more detailed discussion about the changes and the rationale behind them let me know.

BTW: We will take ReSwift-Router in a similar direction.

@kmmraj
Copy link

kmmraj commented Jun 10, 2019

@NemoOudeis - It is interesting to see what has been done, let's discuss these changes. @kaustavjaiswal , @jitinsharma , @hakonber - WDYT?

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

No branches or pull requests

2 participants