Skip to content

fix: Storybook failing to load/update docs#448

Merged
dialmaster merged 2 commits intodevfrom
fix/storybook-msw-worker-setup
Mar 1, 2026
Merged

fix: Storybook failing to load/update docs#448
dialmaster merged 2 commits intodevfrom
fix/storybook-msw-worker-setup

Conversation

@dialmaster
Copy link
Collaborator

@dialmaster dialmaster commented Mar 1, 2026

  • Storybook could not find mockServiceWorker.js; add staticDirs to serve client/public/ where the worker script lives
  • Add postinstall script to auto-generate mockServiceWorker.js on npm install so it doesn't need to be checked into git
  • Update DEVELOPMENT.md Storybook section with prerequisites and remove hardcoded port 6006 reference
  • Fix issues with VideosPage and ChannelManager stories which caused them to crash on render.

- Storybook could not find mockServiceWorker.js; add staticDirs to
  serve client/public/ where the worker script lives
- Add postinstall script to auto-generate mockServiceWorker.js on
  npm install so it doesn't need to be checked into git
- Update DEVELOPMENT.md Storybook section with prerequisites and
  remove hardcoded port 6006 reference
@dialmaster dialmaster requested review from bballdavis and mkulina March 1, 2026 17:53
@claude
Copy link

claude bot commented Mar 1, 2026

PR Review: fix: Storybook failing to load/update docs

Overall this is a clean, well-scoped fix that correctly addresses the root cause of the MSW service worker not being found by Storybook. The approach is sound and consistent with how MSW + Storybook integrations are typically set up.

What looks good

  • Root cause addressed correctly: Adding staticDirs: ['../public'] to main.js is the proper fix. Storybook needs that directory served so the browser can register mockServiceWorker.js via msw-storybook-addon.
  • .gitignore consistency: client/public/mockServiceWorker.js is already gitignored with a helpful regeneration comment — the postinstall approach is consistent with that intent.
  • hasInstallScript: true in package-lock.json is the expected automatic npm change when a postinstall script is added.

Minor suggestions

1. Prefer local binary over npx in postinstall

"postinstall": "npx msw init public/ --save"

Using npx in postinstall works (it will resolve the locally-installed msw first), but invoking the local binary directly is more explicit and avoids any ambiguity:

"postinstall": "node node_modules/.bin/msw init public/ --save"

Or via npx --no-install to ensure it never falls back to downloading a remote package:

"postinstall": "npx --no-install msw init public/ --save"

This is a minor safety/clarity improvement rather than a blocking concern.

2. --save flag is a no-op here

package.json already has the msw.workerDirectory field set:

"msw": {
  "workerDirectory": ["public"]
}

The --save flag would only add that entry if it were missing, so it's harmless but redundant. No action needed.

3. Docs: consider retaining the default port

The original docs had:

Storybook runs on http://localhost:6006.

The replacement:

Storybook will open automatically in your browser.

Storybook does open the browser automatically, but developers often want to know the port to return to it manually (e.g. after accidentally closing the tab, or in environments where auto-open is suppressed). Consider keeping a note like: "Storybook opens automatically at http://localhost:6006" — combining both pieces of information.

Summary

The fix is correct and the changes are minimal and focused. The npx --no-install suggestion is the only one worth considering before merge; the rest are optional polish.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

📊 Test Coverage Report

Backend Coverage

Type Coverage Status
Lines 79.58% 🟡
Statements 79.45% 🟡
Functions 80.70% 🟢
Branches 72.57% 🟡

Frontend Coverage

Type Coverage Status
Lines 87.40% 🟢
Statements 86.71% 🟢
Functions 80.45% 🟢
Branches 81.48% 🟢

Coverage Requirements

  • Minimum threshold: 70% line coverage
  • Backend: ✅ Passes
  • Frontend: ✅ Passes

Coverage report generated for commit 7014f8e

- VideosPage story crashed on render due to missing Router context
  (component uses Link from react-router-dom)
- ChannelManager story crashed on render due to missing Router context
  (component uses useNavigate); MemoryRouter was imported but unused
@claude
Copy link

claude bot commented Mar 1, 2026

PR Review: fix: Storybook failing to load/update docs

This is a focused, well-reasoned fix. The three root causes are each addressed correctly.

Correctness

staticDirs: ['../public'] in main.js — the right fix. Without this, Storybook's dev server doesn't serve public/, so the browser can't register the MSW service worker.

postinstall script — consistent with mockServiceWorker.js being gitignored. Auto-generating on npm install is the correct pattern here. One minor note: prefer npx --no-install msw init public/ --save over bare npx to prevent npm from fetching a remote version of msw if the local one is missing:

"postinstall": "npx --no-install msw init public/ --save"

MemoryRouter decorator in story files — both ChannelManager.story.tsx and VideosPage.story.tsx now wrap stories in MemoryRouter, which is required for any component using react-router-dom hooks. Worth noting: ChannelManager.story.tsx already had the MemoryRouter import before this PR but was missing the decorator — this looks like it was accidentally lost at some point. Good catch restoring it.

Minor observations

  • The --save flag in the postinstall command is redundant — package.json already has the msw.workerDirectory field set — but it's harmless.
  • The docs change removes the hardcoded http://localhost:6006 URL. Developers often need that URL when auto-open is suppressed (e.g., in remote/container environments). Consider: "Storybook opens automatically at http://localhost:6006" to preserve both pieces of info.

Summary

The fix is correct and the scope is appropriately minimal. The postinstall pattern is the standard approach for MSW, the staticDirs addition resolves the service worker 404, and the router decorators fix runtime errors in affected stories. Approve with the --no-install suggestion as a non-blocking improvement.

@dialmaster dialmaster merged commit 7d0f74c into dev Mar 1, 2026
7 checks passed
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