WIP: internal router #61
Conversation
| DPLRouteHandlerBlock routeHandlerBlock = handler; | ||
| routeHandlerBlock(deepLink); | ||
|
|
||
| - (DPLDeepLink *)deepLinkForUrl:(NSURL *)url handler:(id *)handler { |
There was a problem hiding this comment.
I agree that an inout parameter isn't ideal here. Maybe this method could return a dictionary? Or we could even create a lightweight class that had deepLink and handler properties.
There was a problem hiding this comment.
I almost created that lightweight class. IMO is the best option since we don't have tuples in Objective C. :)
|
I like this approach - left a few stylistic suggestions. Didn't get a chance to test it out (on the road right now) but looks good. |
| return NO; | ||
| } | ||
| } | ||
|
|
||
| else if ([handler isKindOfClass:NSClassFromString(@"NSBlock")]) { |
There was a problem hiding this comment.
This might cause a problem in the future if the private class name changes. Can we just create a throwaway DPLRouteHandlerBlock and compare its class to handler's?
There was a problem hiding this comment.
We could certainly do that. That line is actually a copy from what was already in place on the code. I just had to move it to extract things from the methods.
But from what I've seen, we can have two kinds of blocks, with and without a DPLDeepLink argument. So maybe that's why it was this way in the first place?
|
Excited to see a P/R on this! I started this Wiki page to begin to collect a shared set of requirements for using this for internal routing. I think that this is something that is both non-trivial and represents an opportunity to share an opinion on how people should architect and construct their apps. Please do add to it: https://github.com/usebutton/DeepLinkKit/wiki/Internal-DeepLink-Routing |
|
That's really nice, @chrismaddern 😁 Since we don't have a way of commenting on that wiki, I will ask it here:
I have never thought that fetching data could belong to the route handler. Do you think that's a good idea? For instance, how could we present any feedback (progress, connection error) to the user, if you don't have a UI to do it in the handler? I either don't get it or are you planning deeper modifications to the route handler? My intention on this PR is to have direct access to the view controller that will be presented, so we can set some data that wouldn't fit inside the I am open for suggestions on this too. 😉 |
|
What more do you think I can do to so this PR can be considered done? Does this depend on the discussion of the internal router as a whole? |
|
@alcarvalho A good point - I don't think it has to, but I definitely don't want to have to totally change directions in the spirit of keeping non-major updates API-additive vs. breaking. Would adding the deeplink to the viewController creation method solve the problem? |
|
@chrismaddern I am sorry, but I don't follow you. Adding the deeplink to the viewController would solve what problem? |
|
So, could we consider merging the PR? Are there any changes required or suggested before we're able to do it? /cc @chrismaddern @wessmith |
|
triage: needs review |
This is the very first take into trying to extract an internal router as said in #60
I feel the code is a bit cluttered, I don't know. I specially don't like to pass pointers of pointers to anything other then
NSError.Any suggestions on how to improve this?