search

sparsetech / trail

Routing library for the Scala platform
82 stars 8 forks source link

Route: Require exact match when parsing #39

Closed tindzk closed 4 years ago

tindzk commented 4 years ago

Instead of relying on route ordering, a parse should only be successful if no path elements are left. This reverts to the behaviour pre-v0.2.0.

URLs will still parse if they contain parameters or a fragment that were not specified in the route. However, this also can be restricted with the newly introduced function parseArgsStrict().

In cases where remaining path elements or parameters are expected, the matchers Elems and Params were added.

Further changes include renaming of two Route functions:

  1. parse() -> parseArgs()
  2. parseInternal() -> parse()

Since these are potentially breaking changes and may require developers to update their routing tables, the version number was increased to v0.3.0.

Closes #37.

raquo commented 4 years ago

Nice, thanks! One comment: some third parties like to add query params to URLs for tracking ads / marketing / etc. without a way to opt-out, so you might want to reduce the strictness for query params. I guess that would make for a less consistent but a bit more practical-by-default API... not sure about this, just bringing it up in case you haven't considered this.

tindzk commented 4 years ago

Thanks for bringing this up! The same applies to fragments which may be set by the browser and could lead to routes not being parsed in Scala.js. I have added an additional function and reworked the public interface to reflect the new semantics. Please let me know your thoughts.

raquo commented 4 years ago

Looks good, thanks! You might want the route itself to encode whether its query params should be parsed strictly or not (kind of like you do for path elements with Elems, but with the default being non-strict for params) instead of allowing the consuming library a choice of two methods for each route, but that would be more work, and I think I'd wait for someone to actually ask for that. This seems quite fine as-is to me. 👍

tindzk commented 4 years ago

Thanks for your feedback.

Yes, I thought about this too, but opted for the simpler approach of having multiple functions instead. Once the need arises, I'd be happy to make further changes to the interface.