-
-
Notifications
You must be signed in to change notification settings - Fork 1k
fix: navigation respects basepath when no to
was given
#4472
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
base: main
Are you sure you want to change the base?
Conversation
to
was given
View your CI Pipeline Execution ↗ for commit d9c9743.
☁️ Nx Cloud last updated this comment at |
thanks for your PR! I am wondering why this works, given that this user wrote: |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-with-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-plugin
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-plugin
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-server-functions-client
@tanstack/start-server-functions-fetcher
@tanstack/start-server-functions-server
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
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.
This does look OK to me.
@schiller-manuel’s point is interesting. Worth looking into.
@jvllmr please:
- Replicate the tests in the Solid package
- Use unique test names for each run:
test name xyz for $basepath
I wondered about that case as well, but I focused on reproducing the problem with tests and started with the clue from the original issue. I could add the case from the issue to the tests as well just to be sure. I made the test names unique and started on porting the tests to solid as well and pushed my progress for now because I ran out of time for today. |
Yes please. Better to cover our bases with more testing to cover the case. |
Simple fix by passing "." to
Router.resolvePathWithBase
instead of usingfromPath
. Parameterized relative URL tests with basepath to avoid future regressions. I also managed to reproduce the issue this way.fixes #4401