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

Addon-docs: MDX2 support #17515

Merged
merged 11 commits into from
Feb 28, 2022
Merged

Addon-docs: MDX2 support #17515

merged 11 commits into from
Feb 28, 2022

Conversation

shilman
Copy link
Member

@shilman shilman commented Feb 16, 2022

Issue: #17514 #17137

What I did

  • Extract MDXv1 compiler into its own package & clean up snapshot tests
  • Create MDXv2 compiler in its own package
  • Add opt-in mechanism
  • Add MIGRATION docs

How to test

  • CI passes
  • Battery of snapshot tests

@nx-cloud
Copy link

nx-cloud bot commented Feb 16, 2022

☁️ Nx Cloud Report

CI ran the following commands for commit 4db254e. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@shilman shilman marked this pull request as ready for review February 22, 2022 11:48
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a PR for the two packages we should be looking at?

So this means that all apps will depend on @storybook/mdx1-csf and folks that opt in will depend on both? Is that going to be OK from a dependencies point of view?

remarkPlugins: [remarkSlug, remarkExternalLinks],
};

const mdxVersion = global.FEATURES?.previewMdx2 ? 'MDX2' : 'MDX1';
log(`Addon-docs: using ${mdxVersion}`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Console log is intentional?

@shilman
Copy link
Member Author

shilman commented Feb 24, 2022

@tmeasday Yes, there are two more PRs here

The dependency structure is as you described. I don't anticipate any dependency conflicts inside Storybook because each package depends on a specific version and Storybook only depends on the packages. It could, however, cause conflicts with the user's app. This is why we've been prebundling dependencies, and I supppose we could consider prebundling these packages as well. This would require @ndelangen to generalize his prebundling script to be used outside the SB monorepo, and I don't think we need to do it right away.

There is one "hack" which is that SB depends on @mdx-js/react@1.x, which coincidentally works correctly with v2. We could take this PR a step further and also move that dependency into the new packages. However, I didn't do that because:

  1. It would complicate things
  2. I hope this split will be temporary until 7.0, at which point we'll upgrade to MDXv2 completely

@shilman
Copy link
Member Author

shilman commented Feb 28, 2022

@tmeasday @yannbf Merging this to keep things moving forward & reduce merge conflicts. I'll update the dependencies once the mdx-csf PRs are reviewed/merged.

@agrohs
Copy link

agrohs commented Mar 20, 2023

@shilman - wondering if this is documented somewhere around how to use the experimental config w/ mdx2 in storybook 6.5 by chance??

(I'm in a large mono repo that across the board moved to mdx2 and really cannot yet move to storybook 7, so hopeful to get this to run/kick some tires on it! Much appreciate in advance for any help/guidance on it)

@agrohs
Copy link

agrohs commented Mar 20, 2023

Think I may have just found it...is this the correct "guide": https://gist.github.com/shilman/6ff2d7e18db8846e8fc552fb432ae4f6 ??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants