fix(router-core): return meta if match.status notFound #3912
Conversation
packages/router-core/src/router.ts
Outdated
| @@ -1443,7 +1443,7 @@ export class RouterCore< | |||
| // If it's already a success, update headers and head content | |||
| // These may get updated again if the match is refreshed | |||
| // due to being stale | |||
| if (match.status === 'success') { | |||
| if (match.status === 'success' || match.status === 'notFound') { | |||
There was a problem hiding this comment.
while this will invoke the meta fns, the types of those functions are now incorrect I think, as loaderdata is not necessarily defined
There was a problem hiding this comment.
Thanks @schiller-manuel, may you please help with the correct fix for this? We're keen to update to the latest version but this is a blocker for us 🙏🏿
There was a problem hiding this comment.
I am not sure. just making the loader data optional would be the easiest ...
@SeanCassiere why don't we pass in the full AssetFnContextOptions into headers?
There was a problem hiding this comment.
what about any other errors that might be handled by the errorComponent? why specifically just notFound?
There was a problem hiding this comment.
If it always updated the meta tags (success or error) that'd be great! So it should update for success, error and notFound but not redirected or pending?
There was a problem hiding this comment.
probably? unless there is a reason not to update them?
There was a problem hiding this comment.
Nope, will make those changes and thanks for the speedy response!
roduyemi
changed the title
|
View your CI Pipeline Execution ↗ for commit e2b1249.
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/create-router
@tanstack/create-start
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-start
@tanstack/react-router-with-query
@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-plugin
@tanstack/router-generator
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-config
@tanstack/solid-start-client
@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-ssr
@tanstack/start-server-functions-server
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
packages/router-core/src/router.ts
Outdated
| match.headers = route.options.headers?.({ | ||
| loaderData: match.loaderData, | ||
| ...(loaderData && { loaderData }), |
There was a problem hiding this comment.
Why is this bit changing? Same question for the bit down below?
There was a problem hiding this comment.
yeah seems not necessary.
There was a problem hiding this comment.
This is based on your request to make loaderData optional 🙂 is this not what you meant?
There was a problem hiding this comment.
no sorry i was referring to the user facing types
|
please pass |
Thank you for clarifying the change you’re expecting. |
| const headers = route.options.headers?.({ | ||
| loaderData, | ||
| }) | ||
| const headers = route.options.headers?.(assetContext) |
There was a problem hiding this comment.
are the meta functions down here really executed when an error is thrown in the loader? don't we have to run them in the catch block as well?
if my hunch is right, why are your tests not failing?
There was a problem hiding this comment.
The match.meta is updated here as expected but is there another change you're expecting? I can extend the test to add scripts and links etc too if you want?
There was a problem hiding this comment.
I was just wondering what happens if the loader throws. it would not reach those lines, so headers/links/ etc would not get executed, right?
There was a problem hiding this comment.
These lines are executed now because the condition has been updated to include a match.status of error or notFound 🙂
There was a problem hiding this comment.
are you on discord? let's discuss there (just DM me)
There was a problem hiding this comment.
Please let me know if there's anything you'd like me to do to get this moved forward 🙏🏿
Fixes #3657
Updates the match
meta,links,scriptetc. if in case ofmatch.statusbeingnotFoundor an error.