-
-
Notifications
You must be signed in to change notification settings - Fork 908
fix(router-generator): Pathless Layout Route Renders Empty HTML #4003
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?
fix(router-generator): Pathless Layout Route Renders Empty HTML #4003
Conversation
View your CI Pipeline Execution ↗ for commit 415b821.
☁️ Nx Cloud last updated this comment at |
… layout Signed-off-by: leesb971204 <[email protected]>
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-config
@tanstack/react-start-plugin
@tanstack/react-start-router-manifest
@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-config
@tanstack/solid-start-plugin
@tanstack/solid-start-router-manifest
@tanstack/solid-start-server
@tanstack/start
@tanstack/start-api-routes
@tanstack/start-client-core
@tanstack/start-config
@tanstack/start-server-core
@tanstack/start-server-functions-client
@tanstack/start-server-functions-fetcher
@tanstack/start-server-functions-handler
@tanstack/start-server-functions-server
@tanstack/start-server-functions-ssr
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
can you add a test for this? |
also wondering if this maybe needs a fix in the router generator instead? what's the idea of your fix here? |
Just because only a pathless layout file exists doesn’t mean there’s no route. |
yes I think so. the generator currently generates a "virtual route" in this situation. this is then matched at runtime |
Signed-off-by: leesb971204 <[email protected]>
Signed-off-by: leesb971204 <[email protected]>
…dren that are all pathless layouts Signed-off-by: leesb971204 <[email protected]>
I believe the issue can be resolved by updating the |
maybe, I did not look into it deeply yet. can you add an e2e test into one of the existing e2e test setups for react router for this please? then we can check whether the fix solves it |
Got it. But I think additional changes might be needed.
|
…dd updateIsNonPath utility function Signed-off-by: leesb971204 <[email protected]>
I think we'll also need to update the test snapshots on this one. Looking at the current changes on the reproduction sandbox in the reported issue, using the pr-pkg-new packages, the diff shows that a path is being assigned to a pathless layout route. ![]() |
Been thinking about this more and we'd need some comprehensive sandbox (end-to-end) testing for this. Plus, my point earlier about pathless layout routes being assigned a path isn't correct. |
Then, do you think we should approach this in a different way rather than relying on |
…sNonPath function Signed-off-by: leesb971204 <[email protected]>
…yout correctly Signed-off-by: leesb971204 <[email protected]>
…Path Signed-off-by: leesb971204 <[email protected]>
Could you review the last 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.
@leesb971204 its partially resolved.
With your changes, these scenarios now work.
src/routes/
app/
_appLayout.tsx
_appLayout.dashboard.tsx
_appLayout.something.tsx
src/routes/
app/
_appLayout.tsx
_appLayout.dashboard.tsx
_appLayout.something.tsx
_foo.tsx
_foo.index.tsx
However, these two scenarios break the route tree.
- For this scenario, the
routeTree
file gets put into a state where it tries to create an import from'./routes/app'
which of-course does not exist on the file-system.
src/routes/
app/
_appLayout.tsx
_appLayout.dashboard.tsx
_appLayout.something.tsx
_foo.tsx
_foo.something.tsx
- For this scenario, the
routeTree
file adds an additional/app
into the routes. So, instead of it being/app/dashboard
, it instead becomes/app/app/dashboard
.
src/routes/
app/
route.tsx
_appLayout.tsx
_appLayout.dashboard.tsx
_appLayout.something.tsx
_foo.tsx
_foo.index.tsx
I wish I had better feedback to give, as to how you can approach this, but this is something @schiller-manuel and I have discussed earlier at length with the current system being 'best-effort' compromise that we came to.
The shortcut, to get your original sandbox working, would be to throw a redirect
in src/routes/app/index.tsx
to a different route or to set a notFoundComponent
and immediately throw a notFound
.
src/routes/
app/
+ index.tsx
_appLayout.tsx
_appLayout.dashboard.tsx
_appLayout.something.tsx
fixes #3843