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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vite: Move mdx-plugin from @storybook/builder-vite to @storybook/addon-docs #24166

Merged
merged 10 commits into from
Sep 22, 2023

Conversation

bryanjtc
Copy link
Member

@bryanjtc bryanjtc commented Sep 13, 2023

Closes #24058

What I did

Moved the mdx plugin from the vite builder to addon docs.

Checklist for Contributors

Testing

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

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

Manual testing

No manual test needed

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>

@bryanjtc bryanjtc self-assigned this Sep 13, 2023
@bryanjtc bryanjtc changed the title fix(builder-vite): Fix missing @storybook/addon-docs dependency Fix(builder-vite): Fix missing @storybook/addon-docs dependency Sep 13, 2023
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

can we solve this a different way by moving the MDX code out of the vite builder and into addon-docs viteFinal method? @ndelangen

@IanVS
Copy link
Member

IanVS commented Sep 13, 2023

That feels like the right way to do it to me, @shilman.

@ndelangen
Copy link
Member

I agree, @bryanjtc is this something you could do?

@bryanjtc
Copy link
Member Author

sure

@bryanjtc

This comment was marked as resolved.

@bryanjtc bryanjtc changed the title Fix(builder-vite): Fix missing @storybook/addon-docs dependency Fix(builder-vite): Move mdx-plugin from @storybook/builder-vite to @storybook/addon-docs Sep 14, 2023
Copy link
Member

@IanVS IanVS left a comment

Choose a reason for hiding this comment

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

Thanks so much for tackling this! I've wanted to move that plugin out of the vite builder for a long time, just never got around to it.

code/addons/docs/src/plugins/mdx-plugin.ts Outdated Show resolved Hide resolved
@bryanjtc bryanjtc requested a review from IanVS September 18, 2023 16:57
@ndelangen
Copy link
Member

@bryanjtc are you confident that this truly needs no manual regression test?

@bryanjtc
Copy link
Member Author

bryanjtc commented Sep 19, 2023

@bryanjtc are you confident that this truly needs no manual regression test?

The mdx plugin is only used to load mdx in vite projects. Running all vite sandboxes successfully should be enough.

@ndelangen
Copy link
Member

I'll defer to @shilman and @JReinhold to proceed to merge this.

@IanVS
Copy link
Member

IanVS commented Sep 20, 2023

This looks like a good change to me. The only additional thing I can think of that might be worth testing is a vite project that does not have addon-docs installed, since I don't think any of the sandboxes are configured that way. Presumably that wouldn't cause any problems, because then mdx wouldn't be used in the project and this vite plugin wouldn't be needed.

@bryanjtc
Copy link
Member Author

bryanjtc commented Sep 20, 2023

This looks like a good change to me. The only additional thing I can think of that might be worth testing is a vite project that does not have addon-docs installed, since I don't think any of the sandboxes are configured that way. Presumably that wouldn't cause any problems, because then mdx wouldn't be used in the project and this vite plugin wouldn't be needed.

I did a manual test modifying the package.json and main.ts from the react-vite-default-ts sandbox and it works.
I removed addon-docs and addon-essentials from the package.json.
Then removed mdx stories from the main.ts and added the other addons that come with addon-essentials.

main.ts

import type { StorybookConfig } from '@storybook/react-vite';

const config: StorybookConfig = {
  stories: ['../src/**/*.stories.@(js|jsx|mjs|ts|tsx)', {
    directory: '../template-stories/lib/preview-api',
    titlePrefix: 'lib/preview-api',
    files: '**/*.@(stories.@(js|jsx|ts|tsx))'
  }, {
    directory: '../template-stories/addons/links',
    titlePrefix: 'addons/links',
    files: '**/*.@(stories.@(js|jsx|ts|tsx))'
  }, {
    directory: '../template-stories/addons/actions',
    titlePrefix: 'addons/actions',
    files: '**/*.@(stories.@(js|jsx|ts|tsx))'
  }, {
    directory: '../template-stories/addons/backgrounds',
    titlePrefix: 'addons/backgrounds',
    files: '**/*.@(stories.@(js|jsx|ts|tsx))'
  }, {
    directory: '../template-stories/addons/controls',
    titlePrefix: 'addons/controls',
    files: '**/*.@(stories.@(js|jsx|ts|tsx))'
  }, {
    directory: '../template-stories/addons/docs',
    titlePrefix: 'addons/docs',
    files: '**/*.@(stories.@(js|jsx|ts|tsx))'
  }, {
    directory: '../template-stories/addons/toolbars',
    titlePrefix: 'addons/toolbars',
    files: '**/*.@(stories.@(js|jsx|ts|tsx))'
  }, {
    directory: '../template-stories/addons/viewport',
    titlePrefix: 'addons/viewport',
    files: '**/*.@(stories.@(js|jsx|ts|tsx))'
  }, {
    directory: '../template-stories/addons/interactions',
    titlePrefix: 'addons/interactions',
    files: '**/*.@(stories.@(js|jsx|ts|tsx))'
  }],

  addons: [
    '@storybook/addon-links',
    '@storybook/addon-onboarding',
    '@storybook/addon-interactions',
    '@storybook/addon-actions',
    '@storybook/addon-backgrounds',
    '@storybook/addon-controls',
    '@storybook/addon-highlight',
    '@storybook/addon-outline',
    '@storybook/addon-toolbars',
    '@storybook/addon-viewport',
  ],

  framework: {
    name: '@storybook/react-vite',
    options: {},
  },

  docs: {
    autodocs: 'tag',
  },

  previewAnnotations: ['./src/stories/components', './template-stories/lib/preview-api/preview.ts', './template-stories/addons/toolbars/preview.ts'],
  features: {},

  core: {
    disableWhatsNewNotifications: true
  },

  viteFinal: (config) => ({
    ...config,
    optimizeDeps: { ...config.optimizeDeps, force: true },
    server: {
      ...config.server,
      fs: {
        ...config.server?.fs,
        allow: ['stories', 'src', 'template-stories', 'node_modules', ...(config.server?.fs?.allow || [])],
      },
    },
  })
};
export default config;

package.json

{
  "name": "before-storybook",
  "private": true,
  "version": "0.0.0",
  "type": "module",
  "scripts": {
    "dev": "vite",
    "build": "tsc && vite build",
    "lint": "eslint . --ext ts,tsx --report-unused-disable-directives --max-warnings 0",
    "preview": "vite preview",
    "storybook": "NODE_OPTIONS='--preserve-symlinks --preserve-symlinks-main' STORYBOOK_TELEMETRY_URL=\"http://localhost:6007/event-log\" storybook dev -p 6006",
    "build-storybook": "NODE_OPTIONS='--preserve-symlinks --preserve-symlinks-main' STORYBOOK_TELEMETRY_URL=\"http://localhost:6007/event-log\" storybook build"
  },
  "dependencies": {
    "@types/node": "16",
    "react": "^18.2.0",
    "react-dom": "^18.2.0"
  },
  "devDependencies": {
    "@storybook/addon-actions": "^7.5.0-alpha.2",
    "@storybook/addon-backgrounds": "^7.5.0-alpha.2",
    "@storybook/addon-controls": "^7.5.0-alpha.2",
    "@storybook/addon-highlight": "^7.5.0-alpha.2",
    "@storybook/addon-interactions": "^7.5.0-alpha.2",
    "@storybook/addon-links": "^7.5.0-alpha.2",
    "@storybook/addon-measure": "^7.5.0-alpha.2",
    "@storybook/addon-onboarding": "^1.0.8",
    "@storybook/addon-outline": "^7.5.0-alpha.2",
    "@storybook/addon-toolbars": "^7.5.0-alpha.2",
    "@storybook/addon-viewport": "^7.5.0-alpha.2",
    "@storybook/blocks": "^7.5.0-alpha.2",
    "@storybook/jest": "next",
    "@storybook/react": "^7.5.0-alpha.2",
    "@storybook/react-vite": "^7.5.0-alpha.2",
    "@storybook/test-runner": "next",
    "@storybook/testing-library": "next",
    "@types/react": "^18.2.15",
    "@types/react-dom": "^18.2.7",
    "@typescript-eslint/eslint-plugin": "^6.0.0",
    "@typescript-eslint/parser": "^6.0.0",
    "@vitejs/plugin-react": "^4.0.3",
    "eslint": "^8.45.0",
    "eslint-plugin-react-hooks": "^4.6.0",
    "eslint-plugin-react-refresh": "^0.4.3",
    "eslint-plugin-storybook": "^0.6.13",
    "storybook": "^7.5.0-alpha.2",
    "typescript": "^5.0.2",
    "vite": "^4.4.5"
  },
  "packageManager": "yarn@3.6.3",
  "resolutions": {
    "@storybook/addon-a11y": "portal:/home/bryan/projects/storybook/code/addons/a11y",
    "@storybook/addon-actions": "portal:/home/bryan/projects/storybook/code/addons/actions",
    "@storybook/addon-backgrounds": "portal:/home/bryan/projects/storybook/code/addons/backgrounds",
    "@storybook/addon-controls": "portal:/home/bryan/projects/storybook/code/addons/controls",
    "@storybook/addon-mdx-gfm": "portal:/home/bryan/projects/storybook/code/addons/gfm",
    "@storybook/addon-highlight": "portal:/home/bryan/projects/storybook/code/addons/highlight",
    "@storybook/addon-interactions": "portal:/home/bryan/projects/storybook/code/addons/interactions",
    "@storybook/addon-jest": "portal:/home/bryan/projects/storybook/code/addons/jest",
    "@storybook/addon-links": "portal:/home/bryan/projects/storybook/code/addons/links",
    "@storybook/addon-measure": "portal:/home/bryan/projects/storybook/code/addons/measure",
    "@storybook/addon-outline": "portal:/home/bryan/projects/storybook/code/addons/outline",
    "@storybook/addon-storyshots": "portal:/home/bryan/projects/storybook/code/addons/storyshots-core",
    "@storybook/addon-storyshots-puppeteer": "portal:/home/bryan/projects/storybook/code/addons/storyshots-puppeteer",
    "@storybook/addon-storysource": "portal:/home/bryan/projects/storybook/code/addons/storysource",
    "@storybook/addon-themes": "portal:/home/bryan/projects/storybook/code/addons/themes",
    "@storybook/addon-toolbars": "portal:/home/bryan/projects/storybook/code/addons/toolbars",
    "@storybook/addon-viewport": "portal:/home/bryan/projects/storybook/code/addons/viewport",
    "@storybook/builder-manager": "portal:/home/bryan/projects/storybook/code/builders/builder-manager",
    "@storybook/builder-vite": "portal:/home/bryan/projects/storybook/code/builders/builder-vite",
    "@storybook/builder-webpack5": "portal:/home/bryan/projects/storybook/code/builders/builder-webpack5",
    "@storybook/addons": "portal:/home/bryan/projects/storybook/code/deprecated/addons",
    "@storybook/channel-postmessage": "portal:/home/bryan/projects/storybook/code/deprecated/channel-postmessage",
    "@storybook/channel-websocket": "portal:/home/bryan/projects/storybook/code/deprecated/channel-websocket",
    "@storybook/client-api": "portal:/home/bryan/projects/storybook/code/deprecated/client-api",
    "@storybook/core-client": "portal:/home/bryan/projects/storybook/code/deprecated/core-client",
    "@storybook/api": "portal:/home/bryan/projects/storybook/code/deprecated/manager-api-shim",
    "@storybook/preview-web": "portal:/home/bryan/projects/storybook/code/deprecated/preview-web",
    "@storybook/store": "portal:/home/bryan/projects/storybook/code/deprecated/store",
    "@storybook/angular": "portal:/home/bryan/projects/storybook/code/frameworks/angular",
    "@storybook/ember": "portal:/home/bryan/projects/storybook/code/frameworks/ember",
    "@storybook/html-vite": "portal:/home/bryan/projects/storybook/code/frameworks/html-vite",
    "@storybook/html-webpack5": "portal:/home/bryan/projects/storybook/code/frameworks/html-webpack5",
    "@storybook/nextjs": "portal:/home/bryan/projects/storybook/code/frameworks/nextjs",
    "@storybook/preact-vite": "portal:/home/bryan/projects/storybook/code/frameworks/preact-vite",
    "@storybook/preact-webpack5": "portal:/home/bryan/projects/storybook/code/frameworks/preact-webpack5",
    "@storybook/react-vite": "portal:/home/bryan/projects/storybook/code/frameworks/react-vite",
    "@storybook/react-webpack5": "portal:/home/bryan/projects/storybook/code/frameworks/react-webpack5",
    "@storybook/server-webpack5": "portal:/home/bryan/projects/storybook/code/frameworks/server-webpack5",
    "@storybook/svelte-vite": "portal:/home/bryan/projects/storybook/code/frameworks/svelte-vite",
    "@storybook/svelte-webpack5": "portal:/home/bryan/projects/storybook/code/frameworks/svelte-webpack5",
    "@storybook/sveltekit": "portal:/home/bryan/projects/storybook/code/frameworks/sveltekit",
    "@storybook/vue-vite": "portal:/home/bryan/projects/storybook/code/frameworks/vue-vite",
    "@storybook/vue-webpack5": "portal:/home/bryan/projects/storybook/code/frameworks/vue-webpack5",
    "@storybook/vue3-vite": "portal:/home/bryan/projects/storybook/code/frameworks/vue3-vite",
    "@storybook/vue3-webpack5": "portal:/home/bryan/projects/storybook/code/frameworks/vue3-webpack5",
    "@storybook/web-components-vite": "portal:/home/bryan/projects/storybook/code/frameworks/web-components-vite",
    "@storybook/web-components-webpack5": "portal:/home/bryan/projects/storybook/code/frameworks/web-components-webpack5",
    "@storybook/channels": "portal:/home/bryan/projects/storybook/code/lib/channels",
    "@storybook/cli": "portal:/home/bryan/projects/storybook/code/lib/cli",
    "sb": "portal:/home/bryan/projects/storybook/code/lib/cli-sb",
    "storybook": "portal:/home/bryan/projects/storybook/code/lib/cli-storybook",
    "@storybook/client-logger": "portal:/home/bryan/projects/storybook/code/lib/client-logger",
    "@storybook/codemod": "portal:/home/bryan/projects/storybook/code/lib/codemod",
    "@storybook/core-common": "portal:/home/bryan/projects/storybook/code/lib/core-common",
    "@storybook/core-events": "portal:/home/bryan/projects/storybook/code/lib/core-events",
    "@storybook/core-server": "portal:/home/bryan/projects/storybook/code/lib/core-server",
    "@storybook/core-webpack": "portal:/home/bryan/projects/storybook/code/lib/core-webpack",
    "@storybook/csf-plugin": "portal:/home/bryan/projects/storybook/code/lib/csf-plugin",
    "@storybook/csf-tools": "portal:/home/bryan/projects/storybook/code/lib/csf-tools",
    "@storybook/docs-tools": "portal:/home/bryan/projects/storybook/code/lib/docs-tools",
    "@storybook/instrumenter": "portal:/home/bryan/projects/storybook/code/lib/instrumenter",
    "@storybook/manager-api": "portal:/home/bryan/projects/storybook/code/lib/manager-api",
    "@storybook/node-logger": "portal:/home/bryan/projects/storybook/code/lib/node-logger",
    "@storybook/postinstall": "portal:/home/bryan/projects/storybook/code/lib/postinstall",
    "@storybook/preview": "portal:/home/bryan/projects/storybook/code/lib/preview",
    "@storybook/preview-api": "portal:/home/bryan/projects/storybook/code/lib/preview-api",
    "@storybook/react-dom-shim": "portal:/home/bryan/projects/storybook/code/lib/react-dom-shim",
    "@storybook/router": "portal:/home/bryan/projects/storybook/code/lib/router",
    "@storybook/source-loader": "portal:/home/bryan/projects/storybook/code/lib/source-loader",
    "@storybook/telemetry": "portal:/home/bryan/projects/storybook/code/lib/telemetry",
    "@storybook/theming": "portal:/home/bryan/projects/storybook/code/lib/theming",
    "@storybook/types": "portal:/home/bryan/projects/storybook/code/lib/types",
    "@storybook/preset-create-react-app": "portal:/home/bryan/projects/storybook/code/presets/create-react-app",
    "@storybook/preset-html-webpack": "portal:/home/bryan/projects/storybook/code/presets/html-webpack",
    "@storybook/preset-preact-webpack": "portal:/home/bryan/projects/storybook/code/presets/preact-webpack",
    "@storybook/preset-react-webpack": "portal:/home/bryan/projects/storybook/code/presets/react-webpack",
    "@storybook/preset-server-webpack": "portal:/home/bryan/projects/storybook/code/presets/server-webpack",
    "@storybook/preset-svelte-webpack": "portal:/home/bryan/projects/storybook/code/presets/svelte-webpack",
    "@storybook/preset-vue-webpack": "portal:/home/bryan/projects/storybook/code/presets/vue-webpack",
    "@storybook/preset-vue3-webpack": "portal:/home/bryan/projects/storybook/code/presets/vue3-webpack",
    "@storybook/preset-web-components-webpack": "portal:/home/bryan/projects/storybook/code/presets/web-components-webpack",
    "@storybook/html": "portal:/home/bryan/projects/storybook/code/renderers/html",
    "@storybook/preact": "portal:/home/bryan/projects/storybook/code/renderers/preact",
    "@storybook/react": "portal:/home/bryan/projects/storybook/code/renderers/react",
    "@storybook/server": "portal:/home/bryan/projects/storybook/code/renderers/server",
    "@storybook/svelte": "portal:/home/bryan/projects/storybook/code/renderers/svelte",
    "@storybook/vue": "portal:/home/bryan/projects/storybook/code/renderers/vue",
    "@storybook/vue3": "portal:/home/bryan/projects/storybook/code/renderers/vue3",
    "@storybook/web-components": "portal:/home/bryan/projects/storybook/code/renderers/web-components",
    "@storybook/blocks": "portal:/home/bryan/projects/storybook/code/ui/blocks",
    "@storybook/components": "portal:/home/bryan/projects/storybook/code/ui/components",
    "@storybook/manager": "portal:/home/bryan/projects/storybook/code/ui/manager",
    "@vitejs/plugin-react": "^4.0.0"
  }
}

@bryanjtc
Copy link
Member Author

Can you merge this with 7.5.0?

@IanVS IanVS assigned shilman and JReinhold and unassigned bryanjtc Sep 22, 2023
@IanVS
Copy link
Member

IanVS commented Sep 22, 2023

I'd merge it, but it sounds like Norbert wants to leave it up to @shilman or @JReinhold, so I've assigned them.

@ndelangen ndelangen merged commit b308e54 into next Sep 22, 2023
53 checks passed
@ndelangen ndelangen deleted the fix-yarn-pnp branch September 22, 2023 13:10
@ndelangen
Copy link
Member

MERGED 馃帀

@github-actions github-actions bot mentioned this pull request Sep 22, 2023
20 tasks
@JReinhold JReinhold changed the title Fix(builder-vite): Move mdx-plugin from @storybook/builder-vite to @storybook/addon-docs Vite: Move mdx-plugin from @storybook/builder-vite to @storybook/addon-docs Oct 3, 2023
@yannbf yannbf mentioned this pull request Oct 17, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants