raviger
raviger copied to clipboard
kyeotic
Support more characters in path part matchers
The current code for matching path segments to route props is quite simple: the regex: /:[a-zA-Z]+/. In at least one instance this surprised a user. As @coodoo points out This could be expanded to the following valid URL characters
; , / ? : @ & = + $ - _ . ! ~ * ' ( ) #
Since changing this is a breaking change for the library I am opening an issue to invite discussion on the topic.
The way I see it none of the following characters belong in a JS variable, so they should not be part of the route prop matcher.
; , / ? : @ & = + - . ! ~ * ' ( ) #
That leaves: $ _. I think as part of a regex using $ could be confusing, and I also think its probably unlikely to be wanted. That leaves only _. I am willing to create a breaking change to add this, but only once there is critical mass for it. Breaking changes are a cost to the userbase, and I don't want to charge that cost if the benefit is to less than 5% of the users. Of course, I don't have a good way to poll, or even a good idea of how big the user base is, but I do have a good handle on the number of people active on this repository and its about 10. So if I can get 3 people who want a breaking change in order to have underscores in route props I will make the change.
I will leave the issue open for 1 year, or until that happens, whichever comes first. Of course, the floor is still open for anyone that wants to defend one of the characters I didn't like.
Out of curiosity why would adding support _ a breaking change? Wouldn't it be a revise on the regex part will do the trick?
That being said, I'm totally fine with not supporting _, it's just a habit brought from those python days. 🤷🏻♂️
const routes = {
"/users/:userId": ({ userId }) => <div>{userId}</div>,
"/users/:userId_test": ({ userId }) => <div>{userId}</div>
};
currently the second route would match with /users/n33pm_test and userId = n33pm.
after the change userId_test = n33pm_test and userId is undefined and both routes would match the same.
probably it breaks ?!
and back to the question. If we change it I would add a start/end character like /user/:{userId} because something like /users/something:userIdsomething doesn't work and we could deprecate with /:[a-zA-Z]+/g and add an extra /:{[a-zA-Z0-9$_]+}/gU or keep both.
Sorry @coodoo I thought I responded to this, but I must not have hit enter.
It's a breaking change because routes that match with an underscore will get a different variable passed in as a route parameter. That is the front-door API for the entire library, and changing the output like that would cause a previously functioning route to start failing. I know it seems like a minor addition to a regex, but that regex is the API for useRoutes.
If we change it I would add a start/end character like /user/:{userId} because something like /users/something:userIdsomething doesn't work and we could deprecate with /:[a-zA-Z]+/g and add an extra /:{[a-zA-Z0-9$_]+}/gU or keep both.
That is an interesting case, but its rather easy to strip off the beginning of that value inside the route handler. I don't think its worth the complexity to support what is probably a very edge case.
'/users/:userId': ({ userId}) => (<Comp userId={userId && userId.replace('something', '')} />)
@kyeotic Is there any workaround for this one that allows us to still have dots in paths?
@kyeotic Is there any workaround for this one that allows us to still have dots in paths?
Yes, dot is part of the set of characters we can expand to support. But so far there has not been any interest in taking the breaking change in order to add that support.
@kyeotic Understood. Well, I'm all for expanding it as one of the projects I'm working on requires dots in a path part matcher at the moment. :-)
How do you feel about having an option for useRoutes called "expanded path part matching" or so, defaulting to false to not break existing applications?
I think that's a pretty good idea. I'm in the middle of a move currently, so I wont have time for any development for a bit. I would absolutely take a PR for this feature though 😄
@kyeotic Upon closer inspection, it turned out that it wasn't Raviger, but Vite that was causing my problem with dots: https://github.com/vitejs/vite/issues/2245 :dizzy_face:
Nevertheless I wrote a PR anyway - it's coming your way in a minute.
Re-reading the intro again, I no longer agree with my own statement
The way I see it none of the following characters belong in a JS variable, so they should not be part of the route prop matcher.
The important factor isn't whether these characters are valid javascript variable names. That's an implementation detail of the language, and the URL is part of the application's interface. The important factor is whether those characters belong in a URL, and to that end raviger should aim to support everything that the browser supports.
Since this would be a breaking change I plan on taking @rkok's route and adding a route matching parameter to expand character matching support.
Re-reading the intro again, I no longer agree with my own statement
The way I see it none of the following characters belong in a JS variable, so they should not be part of the route prop matcher.
The important factor isn't whether these characters are valid javascript variable names. That's an implementation detail of the language, and the URL is part of the application's interface. The important factor is whether those characters belong in a URL, and to that end raviger should aim to support everything that the browser supports.
Since this would be a breaking change I plan on taking @rkok's route and adding a route matching parameter to expand character matching support.
This comment got distracted by the "URL" part of the request and forgot that it was also a request for "path parts", which are not actually part of the url. God I'm an idiot sometimes.
Matching on all valid URL characters is already possible, and in fact several of them are already in unit tests. This really is a request to support more characters in the part of the route that turns into an actual javascript variable. So, I am only going to add underscores. If you want to use other characters in your variable names, you are free to remap the path part to a local variable of your choosing.