Skip to content

Segment validators#227

Merged
ryansolid merged 6 commits intosolidjs:mainfrom
Breek:segment-validator
Jan 19, 2023
Merged

Segment validators#227
ryansolid merged 6 commits intosolidjs:mainfrom
Breek:segment-validator

Conversation

@jchatard
Copy link
Copy Markdown
Contributor

@jchatard jchatard commented Jan 1, 2023

Hi Ryan,

I have a need for the project I'm currently working on, to have complex path matching rules. To target different routes based on the path.

The mechanism in place allow to match on the shape of the paths. But it's impossible to have 2 different routes for theses 2 paths (path => route which I would express):

  • /product/soap.html => /product/:product_type
  • /product/marvelous-soap-12345.html => /product/:product_slug

I have implemented what I call Segment Validators. I would love to have your opinion on naming and ergonomics.

It's a function which is called to validate the segment value against the validator defined in user-land.

For example in a route definition:

  • :id could be validated as a integer,
  • :controller could either "product" or "cart", but nothing else,
  • :slug could a complexe string that we would test against a regex.

I think this is great for the flexibility of the router, because this allows to have routes which share the same shape /seg1/seg2/seg3 but matching different routes. I stop here, I'm sure you've got the point.

Every previous tests is passing. I've added new ones for this evolution. And I tested in Solid Start, it works out-the-box when you add route definitions in root.tsx.

const segmentValidators: SegmentValidators = {
    number: (v: string) => /^\d+$/.test(v),
    html: (v: string) => /\.html$/.test(v),
  };

<Routes>
  <Route
    path="/:number/:html"
    element={<Test />}
    segmentValidators={segmentValidators}
  />
  <FileRoutes />
</Routes>

I'd love your feedback one this.

Thanks,
Jérémy

@ryansolid
Copy link
Copy Markdown
Member

Actually I almost opened an issue for this. This is in my opinion one of the biggest missing things right now. So I'm very excited by this. Only question really is API. I'd love to look at more prior art to decide on that because this is a feature I haven't had experience with. I've seen sometimes people put the regex right into the string pattern. But I'm not sure that is any good.

@jchatard
Copy link
Copy Markdown
Contributor Author

Yeah, I understand, I think you need to play with it before deciding on the API.

For example, I made several times the mistake to define my validators with a start colon ":"

const segmentValidators: SegmentValidators = {
    ":number": (v: string) => /^\d+$/.test(v),
    ":html": (v: string) => /\.html$/.test(v),
  };

So maybe it's better to with it. I think, DX is really important here.

Thanks.

@leoj3n
Copy link
Copy Markdown

leoj3n commented Jan 11, 2023

Don't know if this is possible but here is an attempt at brainstorming another possibility...

const segmentValidators = {
    number: (v: string) => /^\d+$/.test(v),
    html: (v: string) => /\.html$/.test(v),
  };

<Routes>
  <Route
    path="/:number/archive/:html"
    :number={segmentValidators.number}
    :html={segmentValidators.html}
    element={<Test />}
  />
  <FileRoutes />
</Routes>
<Routes>
  <Route
    path="/:number/archive/:html"
    :number={/^\d+$/}
    :html={/\.html$/}
    element={<Test />}
  />
  <FileRoutes />
</Routes>
Probably not this
const segmentValidators = {
    number: (v: string) => /^\d+$/.test(v),
    html: (v: string) => /\.html$/.test(v),
  };

<Routes>
  <Route
    path={{
      number: segmentValidators.number,
      "archive",
      html: segmentValidators.html
    }}
    element={<Test />}
  />
  <FileRoutes />
</Routes>
<Routes>
  <Route
    path={{
      number: /^\d+$/,
      "archive",
      html: /\.html$/
    }}
    element={<Test />}
  />
  <FileRoutes />
</Routes>

I like that this would be less ambiguous and more coupled, although I wouldn't mind having to use the original suggestion.

@jchatard
Copy link
Copy Markdown
Contributor Author

Yeah, that is very explicit, but I just found that the downside is it requires more code to write. But I like the expressiveness.

@leoj3n
Copy link
Copy Markdown

leoj3n commented Jan 11, 2023

Yeah, that is very explicit, but I just found that the downside is it requires more code to write. But I like the expressiveness.

I guess it depends on how often you expect to reuse the exact matching functions/patterns defined up top for multiple <Route> directives... Passing in the whole segmentValidators object kinda locks you in. I think you would need to break up segmentValidators into multiple objects or use an inline object and end up closer to my suggestion. I imagine it's more likely to want to be specific/granular per <Route> than it is to have one master segmentValidators object.

As far as terminology, it looks like Vue docs refer to this as "Custom regex in params" / "custom regex for a param"... Here we are calling it a "segment validator"... I suppose the difference for us is that it could be a full-fledged function that returns true. Not sure if it makes most sense to call it a "param", "segment", or "part"... Just passing the function without the "SegmentValidator" type kinda avoids this in the code but will still probably have to be given terminology for docs...

I think we can easily first swap "validate" for "match" as in "matching function or regex". As for "segment"...

The docs for Route says things like:

  • "The path segment for this portion of the route." (path segment, route portion)
  • "part of the path" (path part)
  • ":param lets you match an arbitrary name at that point in the path" (path point)

The docs for useParams says things like:

  • "path params of the current route." (path param)
  • "Route params are ... the dynamic parts of the URL" (route param, dynamic URL part)
  • "To access the :id part of the route ... will match the :id part of the URL." (route part, URL part)

Which suggests one of these could work in conjunction with "matching function or regex":

  • path part

  • path point

  • path param

  • path segment

  • route part

  • route param

  • route portion

  • URL part

  • dynamic URL part

It kind of feels like this "matching" behavior has more to do with the idea of a URL/path/route than the actual "params" that get "deserialized" when optionally used in a component. That is to say it has to do more with routing and route matching than it does with the eventual parameters. For this reason I think perhaps there could be a better phrasing than when Vue docs say "custom regex for a param"...

In this case it's probably more of a "custom matching function or regex for a dynamic route path URL part/portion/segment".

Perhaps refined as: "Custom matching function or regex for a dynamic route path segment."

The clarification of dynamic segment and route path terminology might lead one to be more specific in the docs:

Prop Type Description
path string The path segment for this portion of the route.

Dynamic Routes
If you don't know the path ahead of time, you might want to treat part of the path as a flexible parameter that is passed on to the component.

To be more like:

Prop Type Description
path string A route path string that is used to match against the current URL.

Dynamic Route Path Segments
A segment of the route path can be treated as dynamic and accessed as a parameter from the component.

@jchatard
Copy link
Copy Markdown
Contributor Author

I imagine it's more likely to want to be specific/granular per than to have one master segmentValidators object.

Yeah, true! Except for regular things like :id, which would be a number in all routes using an ID...

I think we can easily first swap 'validate' for 'match' as in 'matching function or regex.' As for 'segment'... So it could make sense to re-use the segmentValidator in multiple places.

Agreed for 'match,' it's shorter than 'validate,' and in this context, it has the same meaning.

As for naming, I like 'segment' and 'part' words. I'm always confused when using useParams or {request, params, ...} because of useSearchParams, which deals with the query string. If you come from MDN (emphasis added):

The URLSearchParams interface defines utility methods to work with the query string of a URL.

So 'params' in Solid Start's routing world is always confusing (to me at least). So having a really specific naming for these 'segments' or 'route portions' or 'whatever we decide' feels much needed.

I would even argue that renaming useParams and the naming of those [:params] would be an option, but that's another topic of its own. 😅

@JorrenH
Copy link
Copy Markdown
Contributor

JorrenH commented Jan 12, 2023

As for terminology, I agree with the use of 'matching' in the context of Routes. The feature you're adding adds additional restrictions to the basic path matching syntax, hence you could name them 'matching criteria'.

As for naming the property I would propose 'where' (as used in Laravel) which feels natural in the context of a Route (example below). I do not think the :routeParam property syntax is valid JSX. Especially with named wildcard parameters this becomes difficult ('path/*args').

In terms of path syntax, I wouldn't recomend the Vue route. For consistency it is better to have all your criteria in one place, not split between the path and wherever you put the functional constraints. Moreover, a simple path syntax plays very well with Typescript as it can infer parameter names based on the provided path. A small demo of this can be found here (typescript playground).

For the functional part I mostly agree with the PR, but would like to add some functionality. Besides a predicate on the string literal, I would like passing a RegExp as a shorthand for testing against the string and arrays as an enum. Wildcard routes can also be handled nicely and typesafe. For example:

<Route path="something/:a/:b/:c/*rest" where={{ 
    a: /^\d+$/
    b: ["first", "second",
    c: (s) => s.length > 3,
    rest: (args) => args.length > 2
}}/>

@jchatard
Copy link
Copy Markdown
Contributor Author

@JorrenH I agree with all your points, except that I don’t like the where word.

If you don’t know that this feature exist you’ll never search with where in the docs, unless like you, you come from a Laravel background.

@leoj3n
Copy link
Copy Markdown

leoj3n commented Jan 12, 2023

Typescript as it can infer parameter names based on the provided path

That's very cool... Not sure if it matters but not sure how well the word "matchers" would translate to non-english language.

you’ll never search where in the docs

Perhaps matchFilters would be a better keyword?

<Route path="users/:id/projects/:project" matchFilters={{ 
    id: /^\d+$/,
    project: (s) => !s.startsWith('draft-')
}} />
type MatchFilter =
    | string[]
    | RegExp
    | ((s: string) => boolean)

I'm always confused when using useParams()

Perhaps useRoutePath()?

function User() {
  const routePath = useRoutePath<{ id: string; project: string }>();
 
  // when url is /users/123/projects/hello-world
  console.log(routePath.id, routePath.project);
  // 123, hello-world
}

@leoj3n
Copy link
Copy Markdown

leoj3n commented Jan 12, 2023

Note the docs for https://start.solidjs.com/api/useMatch describes "dynamic route sections" to mean params.

I think a precedent has already been set by existing libraries using "params" to mean any variable part of the URL...

I agree it is not entirely intuitive coming from experience with query params but I can see how they made the leap when naming things like https://reactrouter.com/en/main/hooks/use-params ...

Would be less work to just stick with calling the dynamic route sections param as that's already all over the docs, and this seems to be the convention anyways.

However, I still think useRoutePath() may a worthwhile consideration over useParams() as it is less ambiguous... Perhaps useRouteParams() would be a good more clear option as well alongside the existing useSearchParams().

Yeah so I think personally I'd be leaning towards:

  • where -> matchFilters
  • an object with MatchFilters
  • useParams() -> useRouteParams()
  • "dynamic route sections" (et al.) -> "dynamic route path segments"

@ryansolid
Copy link
Copy Markdown
Member

Thank you. This is exactly the sort of discussion I was looking for. params is basically every client side router. I mostly follow other conventions here although I broke convention by going with <A> tag. But at this point params has sort of sailed.

MatchFilters seems good. One other example I want to throw out there because we need to consider how this gets into file system routing. Here's SvelteKit's version: https://kit.svelte.dev/docs/advanced-routing#matching

They seem to focus on re-usable patterns using a named pattern. Mind you that sort of thing can still be built on top of this. I'm even debating changing the named export in SolidStart more to a route metadata object:

export const routeInfo = {
  data() {},
  matchFilters: {
  
  },
  metadata: {}
}

In that way we can have access to all the route parameters.

Yeah I think I like the direction this discussion is going.

@JorrenH
Copy link
Copy Markdown
Contributor

JorrenH commented Jan 13, 2023

The addition of useRouteParams() would take away some of the confusion of useParams(), which is great. matchFilters do convey their purpose better and would be easier to find in the docs as well, so I would support that.

The approach from SvelteKit is very interesting, and I think named matchFilters would be great to give File routing at least some of the power these filters bring. I'm not too fond of registering named matchers by convention of the directory they are in. Besides, does this work with the treeshaking that happens? Instead I think the following could work well for solid.

// define MatchFilter in 'src/some/file/you/choose.ts'
export const isNumber: MatchFilter = (s: string) => /^\d+$/.test(s);
// use MatchFilter in 'src/components/App.tsx'
import { isNumber } from '../some/file/you/choose';

// use in programmatic routing
<Route path="products/:id" matchFilters={{ id: isNumber }}/>
// use in filerouting
<FileRoutes namedMatchFilters={{ isNumber }}/>
// routes/
// | produts/
// | | [:id=isNumber].tsx

Of course, namedMatchFilters can be named differently, I'm open to suggestions. Additionally, solid-start could provide various common filters as a simple export. Think of things like isNumber, isUUID, isAlpha, isAlphanumeric and so on.

@jchatard
Copy link
Copy Markdown
Contributor Author

I agree for matchFilters.

One other example I want to throw out there because we need to consider how this gets into file system routing. Here's SvelteKit's version: https://kit.svelte.dev/docs/advanced-routing#matching

I like their solution, but (personal taste) I would go for : instead of =. This feels more like Typescript typing:

  • src/routes/archive/[page:integer] meaning page is of type integer
  • src/routes/archive/[page=integer] meaning page is equal to integer

I'm even debating changing the named export in SolidStart more to a route metadata object…

In that way we can have access to all the route parameters.

That’d be great!

But at the same time I also tend to agree with @JorrenH argument:

I'm not too fond of registering named matchers by convention of the directory they are in.

Because (correct me if I’m wrong) Solid Start does not want to impose too much conventions. Adding a property to <Route> or <FileRoutes> is simple enough. But I don’t know, ease of adoption is also key, conventions tend to help.

@ryansolid
Copy link
Copy Markdown
Member

ryansolid commented Jan 13, 2023

I like their solution, but (personal taste) I would go for : instead of =

We can't use : in file systems. The character is reserved for something else.

Because (correct me if I’m wrong) Solid Start does not want to impose too much conventions. Adding a property to or is simple enough. But I don’t know, ease of adoption is also key, conventions tend to help.

In a file system routing system there is no <Route> component only <Routes> so it would need to be in the file or it would be need to be doing the SvelteKit styled name thing in the path and passed into <FileRoutes> I think that's fine. Only reason I mentioned the object is there is a desire for more metadata than just this and I don't want to go and make a ton of named exports. Matching and data functions can't be code split anyway so it makes sense that whole blob could be together. But right now still at a point identifying what sort of things a route file would want to export. The one benefit of the route file export is there is no new = syntax or whatnot. Might be easier for TS too. Tanner has been working on filesystem routing and I've been paying attention.

In any case those are SolidStart questions. Sounds like we are good with the solution here. We should get the PR up to date and look at merging.

@jchatard
Copy link
Copy Markdown
Contributor Author

jchatard commented Jan 15, 2023

Thanks for the contribution @JorrenH 👍

Do you guys think we're good to go?

@ryansolid
Copy link
Copy Markdown
Member

Ok this is great. I think I'm going to merge this and along with the change from the params have us a 0.7.0 release.

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

Successfully merging this pull request may close these issues.

4 participants