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

Customizable URL matching rules #2286

Closed
taion opened this issue Oct 15, 2015 · 23 comments
Closed

Customizable URL matching rules #2286

taion opened this issue Oct 15, 2015 · 23 comments
Labels

Comments

@taion
Copy link
Contributor

taion commented Oct 15, 2015

Lifting this from #2284:


@mjackson

Sure - what I mean is that Flask/Werkzeug offer an example of a good abstraction of pluggable pattern matching (via a mini-DSL) that has worked pretty well for me. The way it looks like is that instead of e.g. our

/a/:foo/b

The preferred syntax for specifying a URL rule looks like e.g.

/a/<foo>/b

This is extensible to allow people to do things like

/a/<str(length=2):foo>/b
/a/<int:foo>/b
/a/<path:foo>/b
/a/<uuid:foo>/b
/a/<my_custom_rule:foo>/b

str, int and path are built-in (in this case the URL rules handle both matching and converting params), while my_custom_rule is an example of the syntax for using a custom URL matching rule. Per the str(length=2) example these converters can also take arguments.

These rules are used to both match and generate URLs.

The nice thing about something like this sort of general solution (not necessarily in this exact syntax form) is that they'd let users solve their own problems, while still keeping the same workflow around route matching, instead of adding something like next() from Express.

ETA: Werkzeug docs are here: http://werkzeug.pocoo.org/docs/0.10/routing/


The <int:foo> case is one decent motivation for having something like this BTW - I've found it a nice convenience to have my URL routing framework just 404 anything that doesn't match the pattern I want before it even gets to my URL handler, and if I'm specifying that it's an integer in the URL rule, I may as well also have the routing convert that parameter to the right type (rather than just a string).

Also I forgot to say, but the difference between str and path above is that path is like the ** glob in that it also matches /s as needed.

@mjackson
Copy link
Member

If this were a small, standalone lib I think I would gladly consider using
it. Up to this point our path matching behavior here has only been dictated
by the needs of users, but it's still pretty basic.

At one point we had this idea to do paramTypes, similar to React's
propTypes, but only for route components.

On Thu, Oct 15, 2015 at 1:58 PM Jimmy Jia notifications@github.com wrote:

Lifting this from #2284

#2284:

@mjackson https://github.com/mjackson

Sure - what I mean is that Flask/Werkzeug offer an example of a good
abstraction of pluggable pattern matching (via a mini-DSL) that has worked
pretty well for me. The way it looks like is that instead of e.g. our

/a/:foo/b

The preferred syntax for specifying a URL rule looks like e.g.

/a//b

This is extensible to allow people to do things like

/a/<str(length=2):foo>/b
/a/int:foo/b
/a/path:foo/b
/a/uuid:foo/b
/a/<my_custom_rule:foo>/b

str, int and path are built-in (in this case the URL rules handle both
matching and converting params), while my_custom_rule is an example of
the syntax for using a custom URL matching rule. Per the str(length=2)
example these converters can also take arguments.

These rules are used to both match and generate URLs.

The nice thing about something like this sort of general solution (not
necessarily in this exact syntax form) is that they'd let users solve their
own problems, while still keeping the same workflow around route matching,
instead of adding something like next() from Express.

ETA: Werkzeug docs are here: http://werkzeug.pocoo.org/docs/0.10/routing/

The int:foo case is one decent motivation for having something like
this BTW - I've found it a nice convenience to have my URL routing
framework just 404 anything that doesn't match the pattern I want before it
even gets to my URL handler, and if I'm specifying that it's an integer in
the URL rule, I may as well also have the routing convert that parameter to
the right type (rather than just a string).

Also I forgot to say, but the difference between str and path above is
that path is like the ** glob in that it also matches /s as needed.


Reply to this email directly or view it on GitHub
#2286.

@taion
Copy link
Contributor Author

taion commented Oct 15, 2015

That makes sense. I might have some time to look at this in a week or so once I get some work deadlines out of the way.

Any thoughts on a reasonable syntax that's compatible with the current syntax for URL parameters?

@mjackson
Copy link
Member

The syntax you have here works well. If something is between <> we know
it's a custom rule. We can still support people who want to use :params and

  • pretty easily. In fact, if this approach is as flexible as you suggest,
    we could probably end up implementing :params and * with it.

On Thu, Oct 15, 2015 at 3:08 PM Jimmy Jia notifications@github.com wrote:

That makes sense. I might have some time to look at this in a week or so
once I get some work deadlines out of the way.

Any thoughts on a reasonable syntax that's compatible with the current
syntax for URL parameters?


Reply to this email directly or view it on GitHub
#2286 (comment)
.

@itajaja itajaja mentioned this issue Oct 28, 2015
3 tasks
@taion taion added this to the v1.1 milestone Oct 28, 2015
@itajaja
Copy link

itajaja commented Oct 29, 2015

We have a new syntax proposal from @th0r regarding the syntax, I personally like it more, as it doesn't require a more advanced DSL than the one that is currently supported and it's more readable (at least in my opinion). Here is an example:

const params = {
    messageId: string({ length: 5 }),
    size: enum(['small', 'medium', 'big'])
};

const routes = (
    /* Common params for the whole app */
    <Route params={params}>
        <Route path="pizza/:size" component={Pizza}/>
        <Route path="message/:messageId" component={Message}/>
        /**
         * We are overriding `size` parameter description for the whole routes branch here:
         * `params` object will be merged with the parent's route `params` here
         */
        <Route path="clothes" component={Clothes} params={{ size: enum(['S', 'M', 'L', 'XL']) }}>
            <Route path="tshirt/:size" component={Tshirt}/>
            /* We can override params per-route if we really need it */
            <Route path="tshirt/big/:size" component={BigTshirt} params={{ size: enum(['XXL', 'XXXL']) }}/>
        </Route>
    </Route>
);

The params attribute can be added to the route to identify the rules for the parameters. This allows to define all the parameter rules at the root level or at the specific route, and any mixing of the two approaches.

@itajaja
Copy link

itajaja commented Nov 6, 2015

Here is an almost finished WIP https://github.com/itajaja/url-matcher

I am not adding it to npm or to Travis, if you decide to port the code to rackt and someone else wants to take ownership. I changed some of the APIs to look a little bit more consistent and robust with rules, and stated clearly what do they return if e.g. a route doesn't match. Let me know what you guys think

@GreenGremlin
Copy link

I would love to see this added. I need to handle routes like "/ca/los-angeles/...", for all states, along side non-state code routes like "/:category/:topic"? I'd like to avoid adding 50 routes, but I know that would work.

@taion
Copy link
Contributor Author

taion commented Nov 12, 2015

We've been a little swamped with getting 1.0.0 out the door and dealing with the immediate aftermath. I'd definitely like to work on getting this integrated when I have a bit more time, though.

@Zoxive
Copy link

Zoxive commented Nov 21, 2015

@itajaja Do you have an idea how url-match would connect with react-router?

Specifically how would react-router <Route> have to be extended to be able to use it? (A new attribute that takes in a func?) Especially since its opt-in only and other react-router users could use other route/url matching libraries as well.

const routes = (
    <Route name="home" path="/" component={RootComponent}>
        <Route name="recordList" /*path=""*/ customMatcher={matchPattern()} component={RecordList} />
        <Route name="record" /*path=""*/ customMatcher={matchPattern()} component={Record} />
    </Route>
);

@itajaja
Copy link

itajaja commented Nov 22, 2015

what about

import * as matcher from 'url-matcher'

<Router matcher={matcher}>
  <Route path="user/:id" component={Pizza} params={{ id: int(max: 8) }}/>
</Router>

Where matcher is an object with the required functions (matchPattern, getRoute, etc).
I don't see a real advantage in having the custom matcher on the Route Level, having different matchers for different routes just looks messy to me.

In this case, params will become a default prop for react-router. Another option would be to pass all the props to the custom matcher so that each matcher can define its props. This would require to change the signature of the current url-matcher module a bit.

@ryanflorence
Copy link
Member

Why not make it all pluggable?

We ship our implementation as the default, but anybody can go nuts:

<Router matcher={fn}/>

It could then pass props.matcher to the useRoutes enhanced history and bob's your uncle:

https://github.com/rackt/react-router/blob/6f8ceac463f49f96bec7d429071c5bd3746c1e7c/modules/useRoutes.js#L52

@taion
Copy link
Contributor Author

taion commented Nov 23, 2015

@ryanflorence

I think for now the best way to go is to do something like with history, where we default to a factored-out version that matches the current behavior, but let users specify overrides.

So by default you'd just get a new Matcher() from url-matcher (like how you get a createHashHistory() from history), but otherwise the "matcher" is an object implementing an appropriate API.

@ryanflorence
Copy link
Member

Or, if that's too much to ask, maybe we just provide a function on a route instead of a path that returns true or false:

<Route match={fn} component={Whatever}/>

@taion
Copy link
Contributor Author

taion commented Nov 23, 2015

Potentially it could make sense to do both - allow specifying something like a function for path on a <Route> and have it be interpreted accordingly.

This would allow routes to fail to match by calling a next() callback - works now that we actually match paths piece-by-piece rather than by concatenating all the path segments.

@taion taion removed this from the v1.1 milestone Dec 21, 2015
@okendoken
Copy link

+1 for

<Route match={fn} component={Whatever}/>

This is something very obvious I expected the router to have

@taion
Copy link
Contributor Author

taion commented Jan 19, 2016

The problem is that a matcher function is not enough – it's break PatternUtils.formatPattern.

@j
Copy link

j commented Feb 18, 2016

Throwing this out there back in my PHP days.... Symfony2's routing just seemed "right"

http://symfony.com/doc/current/book/routing.html#adding-requirements

having a "defaults" and "requirements" property

@arieh
Copy link

arieh commented Feb 28, 2016

Any updates on this issue?
We are updating our site to use react-router but have many legacy URLs with "special characters" (especially parens).

Currently I'm forced to overload history's listen method and strip these characters before they reach the matcher but that scales really badly. It would have been much better to be able to write customized matchers for each route.

@taion
Copy link
Contributor Author

taion commented Feb 28, 2016

See #3105.

@taion
Copy link
Contributor Author

taion commented Jul 5, 2016

Folded into #3611.

@taion taion closed this as completed Jul 5, 2016
@jedwards1211
Copy link

This isn't really a limitation of 4.0.0, right? I'm assuming we could just make custom <Match>-like components that implement this logic.

@eloiqs
Copy link

eloiqs commented Oct 6, 2016

@jedwards1211
Copy link

@eloiqs I don't understand how that relates to what I was saying. Though ambiguous matches are cool

@eloiqs
Copy link

eloiqs commented Oct 6, 2016

@jedwards1211 I may have read this post a bit fast, but I was under the impression that we may be able to implement the kind of logic necessary for matching some URL parameters to specific rule sets by doing something like the example I linked! sorry to bother 🙈

@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests