-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: trailingSlashes option on the type level
#1509
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
Conversation
- always will always add trailing slashes to the leaves of `to` prop - never will always remove trailing slashes from the leaves of `to` prop - preserve will allow both paths with or without slashes
…evel-trailing-slashes
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 3cead7f. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
|
Don't merge yet. I want to add more type tests to cover over parts that could have regressed |
| : 'fullSearchSchema', | ||
| > = PostProcessParams< | ||
| RouteByPath<TRouteTree, TFrom>['types'][TFromRouteType], | ||
| RouteByPath<TRouter['routeTree'], TFrom>['types'][TParamVariant extends 'PATH' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a typescript performance optimization (moving out of the type params)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. I thought about it quite a bit. I now need access to the options on the router. Which would be fine with RegisteredRouter if it weren't for the type tests. Module augmentation is awkward with type tests. It's easier to mock it out if the whole router is a type parameter
|
|
||
| test('when navigating to the root', () => { | ||
| expectTypeOf(Link<RouteTree, string, '/'>) | ||
| type hi = DefaultRouter['options']['trailingSlash'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. Ye. I'm changing the type tests atm
|
Should be ok to merge when approved |
We can infer
trailingSlashesfrom the router and use it to enforce thetoprop when navigating.trailingSlashescurrently has three options: always | never | preserve.always - we always have a trailing slash on leaves autocompleted with to. Branches are not affected in from as you might want a loose match on this for any leaves under a branch instead of an index route.
never - we never have a trailing slash on leaves when autocompleted on to. from does not change as mentioned in always
preserve - to autocompletes to include both trailing slash or without in the union. It can be either.