-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix(docs): broken internal links in the query docs #7247
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit baf7313. 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 1 targetSent with 💌 from NxCloud. |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit baf7313:
|
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #7247 +/- ##
==========================================
- Coverage 41.42% 41.02% -0.41%
==========================================
Files 184 183 -1
Lines 7331 7362 +31
Branches 1531 1541 +10
==========================================
- Hits 3037 3020 -17
- Misses 3889 3933 +44
- Partials 405 409 +4 |
@fulopkovacs Found some other ones in #7248, do you mind including them in yours? |
Oh yeah, sure, thanks for your work! This PR contains like <10% of all the broken links, so I still have a lot more links to fix! 😢 |
@TkDodo I couldn't figure out where should the
So that one is still broken. |
@fulopkovacs in the react docs, this links to the https://tanstack.com/query/v5/docs/framework/react/reference/useQuery in angular, I think we can either remove the link or point it towards the same location as @arnoud-dv FYI |
That would be my preference, I certainly plan to add the API reference. |
Awesome, thanks for the responses. I updated the link to point at |
b854737
to
347cc0a
Compare
Edit: And it's all done. 😅 Ugh, I checked back on this PR, and it looks like I only checked the links for After that, I'll update the description of this PR with the terminal commands I used (spoiler: |
effc8f0
to
a3d7116
Compare
a3d7116
to
85ccfa4
Compare
@TkDodo I think I'm ready with this PR, could you check it please? (The script I used to check the broken links is in the description, under the accordion.) Feel free to tell me if something is missing. |
- [Practical React Query](../../community/tkdodos-blog#1-practical-react-query) | ||
- [React Query as a State Manager](../../community/tkdodos-blog#10-react-query-as-a-state-manager) |
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.
looking at the file structure, we are in:
docs/framework/react/guides/important-defaults.md
and we want to link towards
docs/framework/react/community/tkdodos-blog.md
so imo the correct link should be one level up:
../community/tkdodos-blog
this is also what my editor would auto-complete. So why are two levels up (../../
) needed ?
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.
I'm not sure why the two ..
-s are needed. I just assumed that it's related to the conversation in the in the #router-maintainers
channel in Discord about trailing slashes, and VSCode improperly flagging links with errors, because it did not work the VSCode-way. 🤣 (Still doesn't, I checked.)
I'll ask around in Discord, and come back with the answer.
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.
Here's my question on Discord with my most recent theory on why this solution works... 😅
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.
Okay, I presented this theory to Tanner, and he said it's correct:
- the file is
docs/framework/react/guides/important-defaults.md
- we want to link to
docs/framework/react/community/tkdodos-blog.md
- the relative link that works is
../../community/tkdods.md
My theory on why we need two ..
-s here:
- somewhere along the way the original markdown gets interpreted as the
docs/framework/react/guides/important-defaults/
route .
parent isimportant-defaults
..
grandparent isguides
../..
great-grandparent isreact
Hence ./../../community
takes us to the community
route in the current path's great-grandparent route.
I can't keep up with all the PRs sending fixes for a single broken link so I'll just merge this 😅 |
I found 100+ broken links starting with
/query/v5/docs
with broken-link-checker. This PR fixes them.Most of these come from moving the
core
pages away from theframework
-related pages.TODO
Fix the links for:
How I tested the links
http://localhost:3000
(https://github.com/TanStack/tanstack.com)These commands can be chained if you don't need the output in the terminal. For Angular, this is how the output should look like when all the links are fixed (the
injectQuery.md
page is supposed to be missing, since it'll be added in the future):Testing the broken links with a script