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

Simplify mdxPlugin used in vite builder #19565

Closed
wants to merge 3 commits into from
Closed

Simplify mdxPlugin used in vite builder #19565

wants to merge 3 commits into from

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented Oct 21, 2022

Issue:

We are currently adding @vitejs/plugin-react to every non-react vite storybook project, in order to process the code that is returned from @storybook/mdx2-csf.

What I did

This change pulls in storybookjs/mdx2-csf#20, which swaps out JSX syntax for a call to the _jsx() transform instead, which is what the rest of the code returned from @mdxjs/mdx uses.

That allows us to remove the use of the react vite plugin in our custom mdx vite plugin.

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

I'm not super-confident in this change, and I'm hoping CI will help give a bit of confidence. But it will need a close look as well. I tested in a react sandbox, but the mdx there is not very complex.

@IanVS IanVS added maintenance User-facing maintenance tasks mdx labels Oct 21, 2022
@ndelangen
Copy link
Member

@IanVS is this PR still relevant? I know there's a bunch of other PRs going on regarding MDX, @shilman

@IanVS
Copy link
Member Author

IanVS commented Jan 13, 2023

I think this is cleanup that we'll want to do at some point, yes, but I'm unclear on when it should happen.

@ndelangen
Copy link
Member

@IanVS I think this statement:

We are currently adding @vitejs/plugin-react to every non-react vite storybook project

... isn't true anymore, is it?

What do you propose we do to this PR?

@IanVS
Copy link
Member Author

IanVS commented Nov 30, 2023

I think this is no longer needed.

@IanVS IanVS closed this Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks mdx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants