Skip to content

feat: Upgrade to Storybook 10 with MCP addon support#3839

Draft
lichliter wants to merge 2 commits intoWorkday:prerelease/majorfrom
lichliter:feat/storybook-mcp
Draft

feat: Upgrade to Storybook 10 with MCP addon support#3839
lichliter wants to merge 2 commits intoWorkday:prerelease/majorfrom
lichliter:feat/storybook-mcp

Conversation

@lichliter
Copy link
Copy Markdown

@lichliter lichliter commented Mar 26, 2026

Summary

Upgrade Storybook 8.4.7 → 10.3.3, migrate Yarn Classic → Yarn 4, and add @storybook/addon-mcp for AI-agent-friendly component documentation.

Storybook 10 + Yarn 4

  • Migrate to Yarn 4 with nodeLinker: node-modules
  • Upgrade Storybook to 10.3.3 with ESM-only config loading
  • Restructure .storybook/main.ts for SB10's ESM loader (explicit .ts extensions, JSON import assertions, process.cwd() paths)
  • Add exports fields to workspace packages for ESM compatibility
  • Add Vite resolve.alias entries for workspace package subpath resolution

Re-integrated Vite plugins (via viteFinal)

All custom plugins restored in .storybook/main.ts:

  • vitePluginWholeSource — Show Code / StackBlitz raw source injection
  • vitePluginRedirectMDXToGithub — MDX link rewriting
  • vitePluginInlineSpecifications — spec table pre-parsing
  • vitePluginTypescriptWithTransformers — style transform + docgen pipeline

Emotion ESM patches

Patch 5 @emotion/* packages (css, cache, sheet, weak-memoize, memoize) to add exports fields for Node ESM resolution. Required because SB10's ESM-strict loader can't resolve @emotion/css/create-instance without explicit exports maps. Patches can be removed when Emotion ships native exports support.

MCP manifest enrichment

The @storybook/addon-mcp serves documentation from /manifests/components.json, but Storybook's built-in getCodeSnippet only does single-file AST analysis — it can't follow render: ImportedFn across files, producing stubs like const Basic = () => <Tabs />;.

Fix: An experimental_manifests hook on the config object post-processes the manifest — for each story with an imported render function, it reads the actual example file and replaces the stub snippet with its full source (imports, JSX, hooks, etc.).

Props: Added tsconfig.json paths mapping @workday/canvas-kit-* to source files so the manifest's react-docgen-typescript pipeline resolves components to source instead of node_modules. This extracts real prop tables (e.g. 87 props for Tabs).

Tradeoff: react-docgen-typescript uses the full TypeScript compiler and adds ~30-60s to startup. Acceptable for hosted deployments; can be reverted to reactDocgen: false for local dev.

Known limitations

  • handleFocusRing excluded from styling config — importing it pulls in @workday/canvas-kit-react/common which uses directory subpath exports that Node ESM can't resolve. focusRing() falls back to runtime Emotion (functionally identical, just not statically compiled).
  • Extensionless import warnings — SB10 warns about extensionless imports in workspace packages. These are cosmetic and don't affect functionality.

Release Category

category

Test plan

  • yarn start boots Storybook without errors
  • Components render correctly in browser
  • Show Code panels display full example source
  • Props tables populated in Storybook UI
  • MCP tools return enriched snippets (get-documentation for Tabs shows Tabs.List, Tabs.Item, Tabs.Panel)
  • MCP tools return component props data

@lichliter lichliter changed the title feat: upgrade to Storybook 10 with MCP addon support feat: Upgrade to Storybook 10 with MCP addon support Mar 26, 2026
Migrate from Yarn Classic to Yarn 4 (nodeLinker: node-modules) and
upgrade Storybook 8.4.7 to 10.3.3. Add @storybook/addon-mcp for AI
agent component development, docs, and testing.

Storybook config changes:
- Restructure .storybook/main.ts for SB10's ESM-only config loader
- Add Vite resolve aliases for workspace package subpath resolution
- Add exports fields to workspace packages for ESM compatibility
- ESM-ify imports in .storybook/ plugin files (explicit extensions,
  process.cwd(), JSON import assertions)

Re-integrate all custom Vite plugins via viteFinal:
- vitePluginWholeSource (Show Code / StackBlitz)
- vitePluginRedirectMDXToGithub (MDX link rewriting)
- vitePluginInlineSpecifications (spec table pre-parsing)
- vitePluginTypescriptWithTransformers (style transform + docgen)

Patch 5 @emotion/* packages to add exports fields for Node ESM
resolution of @emotion/css/create-instance and transitive deps.
SB10's ESM-strict loader can't resolve the directory-style subpath
without explicit exports maps. Patches can be removed when Emotion
ships native exports support.

The styling config inlines a simplified version that excludes
handleFocusRing (which would cascade into @workday/canvas-kit-react
directory subpath resolution). focusRing() calls fall back to runtime
Emotion — functionally identical, just not statically compiled.

Made-with: Cursor
@lichliter lichliter force-pushed the feat/storybook-mcp branch from 69a442e to c7c8ae8 Compare March 27, 2026 06:01
Storybook's built-in manifest generator produces stub snippets like
`const Basic = () => <Tabs />;` because getCodeSnippet cannot follow
cross-file imports. This makes the MCP addon's documentation tools
useless for AI consumers.

Fix by attaching an experimental_manifests hook on the config object
that post-processes the manifest: for each story with a `render:`
pointing to an imported example, read the actual example file and
replace the stub snippet with its full source.

Also add tsconfig paths mapping workspace packages to source files
so the manifest's react-docgen-typescript pipeline resolves component
imports to source instead of node_modules (which it rejects).

Tradeoff: react-docgen-typescript uses the full TypeScript compiler
and adds ~30-60s to Storybook startup. This is acceptable for hosted
deployments but can be reverted to `reactDocgen: false` for local
dev if needed.

Made-with: Cursor
@lichliter
Copy link
Copy Markdown
Author

PR Review — Skeptical Maintainer Perspective

Overall impression

This PR bundles three distinct migrations into a single changeset: Storybook 8 → 10, Yarn Classic → Yarn 4, and the new MCP addon integration. Each of these is a non-trivial infrastructure change with its own risk profile. I'd strongly prefer to see these broken into separate PRs (Yarn 4 first, then SB10, then MCP addon) so we can bisect regressions and review each migration on its own terms. As a single PR, the blast radius is enormous and the review burden is disproportionately high.

That said, here are my specific concerns:


1. enrichManifestSnippets is regex-based source parsing — fragile

You're parsing JS/TS imports and export declarations with two regexes:

  • importRe won't match multi-line imports, import type, default imports, or re-exports.
  • renderRe (/export\s+(?:const|var|let)\s+(\w+)[^=]*=\s*\{[^}]*?render:\s*(\w+)/gs) breaks if there's a nested object before render, or if render is defined as an arrow function inline (render: () => <Foo />), or if the story uses satisfies syntax.

A single story file that doesn't match these patterns fails silently. This is the kind of thing that works for 90% of cases and then silently degrades for the rest, with no feedback to the developer. Why not use the TypeScript compiler API you already have in scope, or at least the Storybook CSF parser?


2. experimental_manifests is undocumented internal API

(config as any).experimental_manifests = enrichManifestSnippets;

The name literally has experimental_ in it. The as any cast confirms this isn't part of the public contract. What's the plan when SB 10.x ships a patch that removes or renames this? Is there a tracking issue upstream? If the addon team breaks this, we own the fallout with no deprecation notice.


3. Silent error swallowing in the style transformer

return sourceFile => {
  try {
    return visit(sourceFile);
  } catch {
    return sourceFile;
  }
};

Wrapping the entire style transform in a bare catch that silently returns the unmodified source file is dangerous. If a real bug is introduced in the styling pipeline, this will silently produce un-transformed output with zero indication. At minimum, log the error with the filename so someone has a prayer of debugging it. What caused this to be needed — is there a specific file that throws?


4. Duplicated styling config

The stylingConfig in .storybook/main.ts is a near-exact copy of styling.config.ts minus handleFocusRing. Now we have two copies of the prefix logic, seed generation, and getPrefix function. When someone updates styling.config.ts, they'll need to remember to update this one too — and they won't, because nothing enforces it. Can you extract the shared config into a module that doesn't pull in handleFocusRing?


5. Five Emotion patches are a maintenance liability

Five .yarn/patches/@emotion-* files all adding exports fields. The PR description says "can be removed when Emotion ships native exports support." Is there an upstream issue tracking this? What version of Emotion is expected to ship this? If the answer is "unknown," then we're committing to indefinitely maintaining patches against a dependency we don't control. Every Emotion minor bump is now a potential patch conflict. Please link the upstream issue.


6. .cursor/mcp.json shouldn't be checked in

This hardcodes localhost:9001 and assumes every contributor uses Cursor. This is developer-local config. It should go in .gitignore, with a .cursor/mcp.json.example or a mention in the README for anyone who wants to opt in. Checking it in means every Cursor user on this repo gets an MCP server entry they didn't ask for, and it will error if Storybook isn't running.


7. react-docgen-typescript adds 30-60s to startup

The PR description acknowledges this tradeoff, but there's no escape hatch. The existing SKIP_DOCGEN env var only controls the custom doc parser, not reactDocgen. A contributor running yarn start for a quick visual check now pays an extra 30-60 seconds every time. Can you gate reactDocgen: 'react-docgen-typescript' behind an env var, or at least default to false and only enable it in CI/deployed builds?


8. Catch-all Vite alias is too broad

{
  find: /^(@workday\/canvas-kit-[^/]+)\/([^/]+)$/,
  replacement: '$1/$2/index.ts',
}

This matches any two-segment subpath under any @workday/canvas-kit-* package. What happens when someone imports a subpath where index.ts doesn't exist? This feels like it should be an explicit list of known subpaths rather than a greedy wildcard.


9. enableImmutableInstalls: false in .yarnrc.yml

Disabling immutable installs means CI can silently mutate the lockfile without failing the build. Why is this necessary? If Yarn 4 requires lockfile updates during install, that suggests the lockfile checked in here may not be deterministic. This should be true in CI at minimum.


10. bin field change in modules/mcp/package.json and modules/codemod/package.json

The string form derives the bin name from the name field in package.json. Is this intentionally changing the CLI command name, or an accidental behavioral change? Please confirm.


11. No tests for the new code

The enrichManifestSnippets function is ~60 lines of non-trivial logic (file I/O, regex parsing, map lookups) with no unit tests. The test plan is entirely manual. For a function that will silently degrade if its assumptions break, this needs at least basic coverage: a story file with multi-line imports, re-exports, satisfies syntax, missing example files, etc.


Minor nits

  • DocsPage was removed from preview.js — is that a SB10 deprecation or an intentional removal?
  • The @storybook/csf dependency removal from modules/docs/package.json — is nothing importing from it?
  • The readFileSync calls in enrichManifestSnippets are synchronous I/O in a potentially hot path. How many story files does this iterate over?

Summary

The MCP addon integration itself is a good idea and the manifest enrichment solves a real gap. But this PR couples it with two large infrastructure migrations, uses fragile regex parsing where AST tools are already available, relies on an explicitly experimental API, and introduces silent error swallowing. I'd want to see the migrations split, the enrichManifestSnippets logic hardened (or tested), and the experimental_manifests situation documented with an upstream issue before approving.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant