-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Reference dynamic route variables in Legacy Collection examples in v5.mdx #9555
Conversation
….mdx As dynamic routes are also allowed to use variables, it's easy to overlook when following directions specific to file content. This PR explicitly calls out changing any references to `slug` used in dynamic routes to `id`.
✅ Deploy Preview for astro-docs-2 ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Thanks for this @cdvillard , it's a great catch!
I'm not sure this is the solution, though, because people wouldn't normally be writing file names in Astro code. You made me think hard about whether there's a diff it makes sense to show, and the only thing I can come up with is using our <FileTree>
component to show files!
I'll ask @hippotastic if he's ever tried to render a file diff somehow! 😅
Otherwise, then I think probably not showing a diff for the file name, but rather just mentioning it in the text above, maybe like...
Change references from slug to id. Content layer collections do not have a slug field. Instead, all updated collections will have an id. You may also need to update your file name to match an updated
getStaticPaths()
parameter:
Hehe, interesting idea! :) We're currently not able to show a before/after diff of the file name inside an open file tab. I really like the I therefore think we'll need to resort to regular text to explain the necessary changes for now as you suggested. |
@sarah11918 Took your recommendation into account, though I also renamed the file as well to reflect the change as well. Would that work, instead of worry about illustrating it via a diff? |
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 think this works great now. Thank you @cdvillard and welcome to Team Docs! 🥳
Description (required)
As dynamic routes are also allowed to use variables, it's easy to overlook when following directions specific to file content. This PR explicitly calls out changing any references to
slug
used in dynamic routes toid
.Related issues & labels (optional)
improve documentation
,hacktoberfest-accepted
If you have additional questions, I'm also
cdvillard
in the Discord server.