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

Make Router generic over its route 🚏 #240

Merged
merged 1 commit into from
Jul 30, 2021
Merged

Conversation

p4checo
Copy link
Member

@p4checo p4checo commented Jul 25, 2021

Checklist

Motivation and Context

Our Router and RouteHandler infrastructure was designed to work with plain URL's on its main APIs. Despite URLs being the critical piece of information to route (e.g. in our TrieRouter), in certain situations the URL doesn't contain all the information, which limits the Routing infrastructure capabilities. One such example would be in a server middleware router, where the HTTP method would likely be required information to pass on to handlers to perform adequate routing.

To remove this restriction, a new Routable protocol was created to represent a routable type, which only needs to provide a url and is forwarded by the router to the handler upon a successful match. To make it easier to unpack any additional information on the handlers and have more type safety, it was made an associatedtype of both Router and RouteHandler, effectively turning it into a new generic R.

Description

  • Add new Routable protocol, add default conformance to URL.

  • Add new associatedtype R: Routable to Router and RouteHandler.

  • Update Router and RouterHandler APIs to receive a route of type R.

  • Add new RouteHandler.eraseToAnyRouteHandler helper.

Our `Router` and `RouteHandler` infrastructure was designed to work
with plain `URL`'s on its main APIs. Despite URLs being the critical
piece of information to route (e.g. in our `TrieRouter`), in certain
situations the URL doesn't contain all the information, which limits
the Routing infrastructure capabilities. One such example would be in
a server middleware router, where the HTTP method would likely be
required information to pass on to handlers to perform adequate routing.

To remove this restriction, a new `Routable` protocol was created to
represent a routable type, which only needs to provide a `url` and is
forwarded by the router to the handler upon a successful match. To make
it easier to unpack any additional information on the handlers and have
more type safety, it was made an `associatedtype` of both `Router` and
`RouteHandler`, effectively turning it into a new generic `R`.

## Changes

- Add new `Routable` protocol, add default conformance to `URL`.

- Add new `associatedtype R: Routable` to `Router` and `RouteHandler`.

- Update `Router` and `RouterHandler` APIs to receive a route of type
`R`.

- Add new `RouteHandler.eraseToAnyRouteHandler` helper.
@codecov
Copy link

codecov bot commented Jul 25, 2021

Codecov Report

Merging #240 (120be87) into master (61bbc21) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #240   +/-   ##
=======================================
  Coverage   94.94%   94.94%           
=======================================
  Files          96       97    +1     
  Lines        3264     3266    +2     
=======================================
+ Hits         3099     3101    +2     
  Misses        165      165           
Impacted Files Coverage Δ
Sources/DeepLinking/Routable.swift 100.00% <100.00%> (ø)
Sources/DeepLinking/Route+TrieRouter.swift 91.78% <100.00%> (ø)
Sources/DeepLinking/Router.swift 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 61bbc21...120be87. Read the comment docs.

@p4checo p4checo merged commit aef7b54 into master Jul 30, 2021
@p4checo p4checo deleted the generic-route-in-router branch July 30, 2021 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants