-
-
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
Migrate: skip the automigration for gf markdown when user isn't using mdx #22186
Conversation
…automigration for gf markdown
@@ -22,9 +25,27 @@ export const mdxgfm: Fix<Options> = { | |||
return null; | |||
} | |||
|
|||
const hasMDXFiles = await mainConfig?.stories?.reduce(async (acc, item) => { |
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 wonder if you could just do the same as this automigration: https://github.com/storybookjs/storybook/blob/next/code/lib/cli/src/automigrate/fixes/mdx-1-to-2.ts#L49
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.
That seems worse? considering it's not using the glob patterns set in main.ts
, and will ignore any MDX files not following the .stories.mdx
pattern.
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.
Alright sounds good, maybe the other automigration should follow this pattern then
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.
LGTM!!
Migrate: skip the automigration for gf markdown when user isn't using mdx
Closes #22167
What I did
I added a check if the user is actually using MDX files.
If they do not.. we should skip the automigration.
How to test
Run the automigration on a project that has no MDX files