-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Autodocs: Fix docs pages ignoring meta.id
when calculating their ID
#23520
Autodocs: Fix docs pages ignoring meta.id
when calculating their ID
#23520
Conversation
The second commit includes a fix for the case of MDX docs. It is slightly more involved than the case of autodocs since MDX docs, when being processed, don't have a direct access to I decided to store the whole I am not sure the decisions I made for the second commit are sound, so yes definitely need some reviews! Thanks. |
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.
Let's drop the meta again in the getIndex
, maybe something like:
// Drop the meta as it isn't part of the index, we just used it for record keeping
const { meta, ...existing } = indexEntries[entry.id];
Also, can we add a test case to storyIndexGenerator.test.ts
that uses a component id?
@tmeasday Hello, thanks for the review! I very much appreciate it. I pushed additional commits reflecting what you suggested. I decided to filter out the meta before returning in I'm now going to work on the test cases and I'll let you know when I'm done! Thanks. |
@tmeasday I added two test cases (for autodocs and mdx docs) under Let me know if you prefer other directory/file structures for mock data. Thanks! |
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.
Great job @sookmax!
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.
Looks good! Only a minor request regarding the types.
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.
Looks great!
@sookmax Looks like some merge-conflicts happened, do you think you'd be able to resolve those? |
@ndelangen Yes sure! It doesn't appear to be too complex so let me look into it, and I'll let you know when resolved! |
@ndelangen Merge is done! |
…prioritize-id-over-title-docs
63a45d2
to
c334641
Compare
@ndelangen the previous merge seemed accidentally undo 1e90354 😅. So I re-merged and force-pushed the new merge. |
Meta.id
over Meta.title
for ids of docs pages
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.
Great work! 💪
I'm currently working on a big overhaul of StoryIndexGenerator
, and I'll make sure to merge your changes into that.
@JReinhold Ah, is that related to #23457? Looking forward to learning the new architecture when it's done! 👍 |
Meta.id
over Meta.title
for ids of docs pagesmeta.id
when calculating their ID
Closes #23394
What I did
I made a little adjustment in
StoryIndexGenerator
so that the docs pages that are automatically generated would also prioritizeMeta.id
overMeta.title
ifMeta.id
was provided.I referenced code section below for the adjustment.
storybook/code/lib/csf-tools/src/CsfFile.ts
Lines 469 to 493 in c8e9caf
How to test
yarn task --task sandbox --start-from auto --template react-vite/default-ts
id
tometa
object inButton.stories.ts
(e.g.,MyButton
)Button
docs page and confirm the url is something like:http://localhost:6007/?path=/docs/mybutton--docs
Checklist
MIGRATION.MD
Maintainers
ci:normal
,ci:merged
orci:daily
GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.ts
["cleanup", "BREAKING CHANGE", "feature request", "bug", "build", "documentation", "maintenance", "dependencies", "other"]