Skip to content
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

docs: remove the invalid context and location arguments from being shown as available in the loaderDeps #2091

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

SeanCassiere
Copy link
Member

We haven't had the context and location arguments present as loaderDeps options, so this corrects this error in Router's documentation.

The reason we don't have location as an argument of loaderDeps is quite simple. If the location changes, the loader would anyways have to be refired (or retrieved from a cache using something like Query). The scenario in which this wouldn't be the case, is if you wanted to prevent this from happening when search params change. This is why only search is provided as its own argument.

The reason we don't have context as an argument of loaderDeps is because, when the root context has been changed, the user is expected to call router.invalidate(). This invalidate call discards all cached data and re-evaluates the entire route-tree. As such, having context as an argument in loaderDeps, makes no sense, since it wouldn't actually contribute anything towards the actual loaderDeps functionality.

@SeanCassiere SeanCassiere marked this pull request as ready for review August 6, 2024 08:16
Copy link

nx-cloud bot commented Aug 6, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 507c608. 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 targets

Sent with 💌 from NxCloud.

Copy link

pkg-pr-new bot commented Aug 6, 2024

commit: 507c608

pnpm add https://pkg.pr.new/@tanstack/history@2091
pnpm add https://pkg.pr.new/@tanstack/react-cross-context@2091
pnpm add https://pkg.pr.new/@tanstack/react-router@2091
pnpm add https://pkg.pr.new/@tanstack/react-router-with-query@2091
pnpm add https://pkg.pr.new/@tanstack/router-cli@2091
pnpm add https://pkg.pr.new/@tanstack/router-devtools@2091
pnpm add https://pkg.pr.new/@tanstack/router-generator@2091
pnpm add https://pkg.pr.new/@tanstack/router-plugin@2091
pnpm add https://pkg.pr.new/@tanstack/router-vite-plugin@2091
pnpm add https://pkg.pr.new/@tanstack/start@2091
pnpm add https://pkg.pr.new/@tanstack/start-vite-plugin@2091

Open in Stackblitz

More templates

@SeanCassiere SeanCassiere merged commit 2423d02 into main Aug 6, 2024
5 checks passed
@SeanCassiere SeanCassiere deleted the docs-1216 branch August 6, 2024 08:18
@waweber
Copy link
Contributor

waweber commented Aug 6, 2024

@SeanCassiere is the loader intended to be fired when any property of location changes? (I assume the whole object is replaced on navigate.)

I noticed the route loader did not appear to fire when only location.hash was changed, so I'm not sure if I missed something or if that's a bug.

@SeanCassiere
Copy link
Member Author

@SeanCassiere is the loader intended to be fired when any property of location changes? (I assume the whole object is replaced on navigate.)

I noticed the route loader did not appear to fire when only location.hash was changed, so I'm not sure if I missed something or if that's a bug.

Please open a new issue for this with a reproduction. This is sounding like a bug, but I can't be sure till someone investigates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants