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

Docs: Removal of CSF in MDX format #25609

Merged
merged 10 commits into from
Jan 19, 2024
Merged

Docs: Removal of CSF in MDX format #25609

merged 10 commits into from
Jan 19, 2024

Conversation

jonniebigodes
Copy link
Contributor

Follows up on #25303

What I did

With this pull request, the documentation was updated to reflect the recent Storybook 8 regarding documentation, MDX support and removing the .stories.mdx format support

What was done:

  • Updated the documentation, including some of the readmes (the addon docs README will not feature in this pull request as it will feature in another body of work to streamline the monorepo's READMES)
  • Updated snippets
  • Created some auxiliary snippets to aid the change

@yannbf or @JReinhold, when you have a moment, can you take a pass at this and let me know of any feedback you may have? Thanks in advance.

Checklist for Contributors

Testing

  1. Follow the steps in the contributing instructions for this branch, docs_remove_stories_mdx.
  2. Open the relevant documentation and verify the feature changes.

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

@jonniebigodes jonniebigodes added documentation ci:docs Run the CI jobs for documentation checks only. labels Jan 15, 2024
@jonniebigodes jonniebigodes self-assigned this Jan 15, 2024
@@ -12,7 +12,7 @@ In addition, you can write pure documentation pages in MDX and add them to Story

<Callout variant="info">

Writing stories directly in MDX was deprecated in Storybook 7. Please reference the [previous documentation](../../../release-6-5/docs/writing-docs/mdx.md) for guidance on that feature.
Writing stories directly in MDX was removed in Storybook 8, and we're no longer supporting it. Please reference the [previous documentation](../../../release-6-5/docs/writing-docs/mdx.md) for guidance on that feature or [migrate](../migration-guide.md) to the new format.
Copy link
Contributor

Choose a reason for hiding this comment

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

The migration guide will have lots of irrelevant info for this task, so we should link to a particular heading. I'll make sure this anchor still works in the 8.0 guide, too.

Suggested change
Writing stories directly in MDX was removed in Storybook 8, and we're no longer supporting it. Please reference the [previous documentation](../../../release-6-5/docs/writing-docs/mdx.md) for guidance on that feature or [migrate](../migration-guide.md) to the new format.
Writing stories directly in MDX was removed in Storybook 8, and we're no longer supporting it. Please reference the [previous documentation](../../../release-6-5/docs/writing-docs/mdx.md) for guidance on that feature or [migrate](../migration-guide.md#storiesmdx-to-mdxcsf) to the new format.

<!-- prettier-ignore-end -->

Update your Storybook configuration (in `.storybook/main.js|ts`), and provide the `legacyMdx1` feature flag to enable MDX 1 support.
If you're still writing stories directly in MDX, with the `stories.mdx` format, we're no longer supporting it and recommend migrating to the new format to avoid issues, as the majority of APIs and [Doc Blocks](./doc-blocks.md) used by Storybook were overhauled to support the latest MDX release (e.g., the [`Meta`](../api/doc-block-meta.md) block). To help you transition to the new format, we provide a migration helper in our CLI to automate the process.

<!-- prettier-ignore-start -->

<CodeSnippets
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would we not include this snippet in the callout at the top of the page?

@@ -386,42 +388,23 @@ By applying this pattern with the Controls addon, all anchors will be ignored in

### The MDX documentation doesn't render in my environment

As Storybook relies on MDX 2 to render documentation, some technical limitations may prevent you from migrating to this version. If that's the case, we've prepared a set of instructions to help you transition to this new version.
As Storybook relies on the latest version of [MDX](https://mdxjs.com/) to render documentation, some technical limitations may prevent you from migrating to this version. If that's the case, we've prepared a set of instructions to help you transition to this new version.

#### Storybook doesn't create documentation for my component stories

If you run into a situation where Storybook is not able to detect and render the documentation for your component stories, it may be due to a misconfiguration in your Storybook. Check your configuration file (i.e., `.storybook/main.js|ts`) and ensure the `stories` configuration element provides the correct path to your stories location(e.g., `../src/**/*.stories.@(js|jsx|mjs|ts|tsx)`).

#### The documentation doesn't render using `stories.mdx`
Copy link
Contributor

Choose a reason for hiding this comment

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

Because we've removed support for *.stories.mdx entirely, I don't think the failure path is "the documentation doesn't render". Rather, it's likely "the Storybook itself errors" (need to confirm).

Thus, I think we can just remove this section entirely. The callout at the top is sufficient for those still using it, imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left this as a way to have some localized information on what's happening and provide the right command, as we currently don't have it, and we're in a state where the migration guide is still being updated. I much rather would like for this to live here for now and once we've finalized the migration guide, remove the section from here and use the snippets in the migration guide. Wouldn't you agree?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather include the command in the callout at the top of the page and be done with it.

Copy link

@chantastic chantastic left a comment

Choose a reason for hiding this comment

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

I don't have any additional questions beyond what Kyle brought up. But I also don't have strong opinions on how to proceed. So I'm reviewing as approved contingent of those items being resolved.

Copy link
Contributor

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

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

Only a minor suggestion from me, everything else looks fine.

@@ -386,42 +388,23 @@ By applying this pattern with the Controls addon, all anchors will be ignored in

### The MDX documentation doesn't render in my environment

As Storybook relies on MDX 2 to render documentation, some technical limitations may prevent you from migrating to this version. If that's the case, we've prepared a set of instructions to help you transition to this new version.
As Storybook relies on the latest version of [MDX](https://mdxjs.com/) to render documentation, some technical limitations may prevent you from migrating to this version. If that's the case, we've prepared a set of instructions to help you transition to this new version.
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on what I see here https://github.com/storybookjs/storybook/pull/25303/files#diff-59572a08f167cee7f0025a46c200039f0e97dc011eb05ce32593e5f5d715615cR102 , "latest" is not correct, "MDX 3" would be. When MDX 4 gets released this documentation would be wrong, and upgrading to MDX 4 would most likely be a breaking change to Storybook happening in a major version.

Suggested change
As Storybook relies on the latest version of [MDX](https://mdxjs.com/) to render documentation, some technical limitations may prevent you from migrating to this version. If that's the case, we've prepared a set of instructions to help you transition to this new version.
As Storybook relies on [MDX 3](https://mdxjs.com/) to render documentation, some technical limitations may prevent you from migrating to this version. If that's the case, we've prepared a set of instructions to help you transition to this new version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JReinhold I left this temporarily as a reminder for it to be updated in a follow-up pull request, as there's still some work to be done that's not part of this pull request. Appreciate the consideration nonetheless 🙏

@jonniebigodes jonniebigodes merged commit da24504 into next Jan 19, 2024
18 checks passed
@jonniebigodes jonniebigodes deleted the docs_remove_stories_mdx branch January 19, 2024 12:50
@github-actions github-actions bot mentioned this pull request Jan 19, 2024
18 tasks
@@ -4,7 +4,7 @@

<!-- **NOTE:** As of Storybook 7.2, `@storybook/addon-themes` ships in `@storybook/addon-essentials`. If you're using Storybook >= 7.2, skip to ["Import Bootstrap"](#🥾-import-bootstrap). -->
Copy link
Member

Choose a reason for hiding this comment

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

@jonniebigodes if this is merged into next, shouldn't the note be removed as this all now refers to SB8?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call on this @yannbf, I'll check it and update this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:docs Run the CI jobs for documentation checks only. documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants