✨ Add Storybook Client SDK with Puppeteer (VIZ-59)

[Robdel12]

Summary

Implements @vizzly-testing/storybook - the first official Vizzly CLI plugin that auto-discovers and captures screenshots of Storybook stories using Puppeteer.

Closes #79 (VIZ-59)

What's New

Storybook Plugin (clients/storybook/)

Core Features:

• 🔍 Auto-discovery from Storybook index.json (v6/v7/v8 support)
• 📱 Multi-viewport screenshot capture
• 🧩 Functional programming architecture
• ⚙️ Global and per-story configuration
• 🎯 Interaction hooks (beforeScreenshot)
• 🔀 Pattern-based story filtering (include/exclude)
• ⚡ Parallel processing with configurable concurrency
• 📸 Vizzly SDK integration for screenshot upload

Package Structure:

• Plugin auto-registration via vizzly.plugin field
• Functional modules: config, crawler, browser, screenshot, hooks
• ✅ 52 passing tests with full coverage
• 📚 Comprehensive documentation with examples

CI/CD Enhancements

Optimized Workflows:

• ✅ Path-based conditional execution for client tests
• ✅ Ruby and Storybook clients only test when their files change
• ✅ Saves GitHub Actions minutes
• ✅ Smart ci-check handles conditional job results
• ✅ Release workflow for automated npm publishing

Usage

CLI

npm install @vizzly-testing/storybook

# Capture screenshots from static Storybook
vizzly storybook ./storybook-static

# With custom viewports
vizzly storybook ./storybook-static \
  --viewports "mobile:375x667,desktop:1920x1080"

Programmatic

import { run } from '@vizzly-testing/storybook';

await run('./storybook-static', {
  viewports: 'mobile:375x667,desktop:1920x1080',
  concurrency: 3,
});

Per-Story Config

export let Primary = {
  parameters: {
    vizzly: {
      viewports: [{ name: 'mobile', width: 375, height: 667 }],
      beforeScreenshot: async (page) => {
        await page.hover('button');
      },
    },
  },
};

Testing

• ✅ All 52 tests passing
• ✅ Linting clean
• ✅ Build successful
• ✅ CI workflow validates on Node 20, 22, 24

Next Steps

• Manual integration test with real Storybook build
• First release to npm via release workflow
• Update main CLI documentation
• Announcement - first official plugin! 🎉

Test Plan

1. Automated tests run in CI for all Node versions
2. Manual testing with sample Storybook project
3. Verify plugin auto-discovery works
4. Test all configuration options

[claude]

PR Review: Storybook Plugin Implementation

Overview

This PR implements the first official Vizzly plugin for Storybook screenshot testing. The implementation is well-structured, follows functional programming principles, and includes comprehensive test coverage. Overall, this is high-quality work that's ready to merge with a few minor recommendations.

---

✅ Strengths

Architecture & Code Quality

• Excellent functional programming design: Pure functions, clear separation of concerns across modules (browser, crawler, config, hooks, screenshot)
• Clean plugin architecture: Proper integration with Vizzly's plugin system using the vizzly.plugin field
• Strong type safety: Comprehensive JSDoc comments throughout
• Good error handling: Try-catch blocks with meaningful error messages, graceful fallbacks (e.g., waitForStoryReady)
• Resource management: Proper cleanup with finally blocks for browser/page instances

Configuration System

• Well-designed priority system: CLI options > config file > defaults > per-story config
• Flexible viewport configuration: Both string parsing ("mobile:375x667") and object format
• Per-story customization: parameters.vizzly allows story-level overrides
• Pattern-based hooks: interactions object with glob matching is elegant and powerful

Testing

• 52 tests with good coverage: Unit tests for all major functions
• Edge case handling: Tests for invalid viewports, missing configs, v6/v7 format differences
• Multiple Storybook versions: Support for v6, v7, and v8 is well-tested

CI/CD Enhancements

• Smart path-based execution: Only runs client tests when relevant files change, saving CI minutes
• Proper conditional logic: The ci-check job correctly handles optional job results
• Release automation: Workflow includes Claude-powered changelog generation

---

⚠️ Issues & Recommendations

1. Missing Input Validation in URL Generation (Low severity)

Location: clients/storybook/src/crawler.js:124-126

export function generateStoryUrl(baseUrl, storyId) {
  return `${baseUrl}/iframe.html?id=${encodeURIComponent(storyId)}&viewMode=story`;
}

Issue: No validation that baseUrl or storyId are non-empty strings. Could generate malformed URLs.

Recommendation:

export function generateStoryUrl(baseUrl, storyId) {
  if (!baseUrl || typeof baseUrl !== 'string') {
    throw new Error('baseUrl must be a non-empty string');
  }
  if (!storyId || typeof storyId !== 'string') {
    throw new Error('storyId must be a non-empty string');
  }
  return `${baseUrl}/iframe.html?id=${encodeURIComponent(storyId)}&viewMode=story`;
}

2. Potential Path Traversal in Config File Loading (Low severity)

Location: clients/storybook/src/config.js:34-46

The loadConfigFile function uses cosmiconfig which should handle path traversal safely, but when a custom configPath is provided via CLI, there's no validation.

Recommendation: Add path validation before loading:

export async function loadConfigFile(configPath) {
  let explorer = cosmiconfig('vizzly-storybook');

  try {
    if (configPath) {
      // Validate that path is safe before loading
      const resolvedPath = resolve(configPath);
      if (!resolvedPath.startsWith(process.cwd())) {
        throw new Error('Config file must be within project directory');
      }
    }
    
    let result = configPath
      ? await explorer.load(configPath)
      : await explorer.search();

    return result?.config || {};
  } catch (error) {
    return {};
  }
}

Note: This is defensive - cosmiconfig likely handles this, but explicit validation is safer for security-conscious tools.

3. Missing Timeout Configuration (Low severity)

Location: clients/storybook/src/browser.js:54-58

export async function navigateToUrl(page, url, options = {}) {
  await page.goto(url, {
    waitUntil: 'networkidle2',
    ...options,
  });
}

Issue: No timeout specified - could hang indefinitely on slow/broken stories.

Recommendation:

export async function navigateToUrl(page, url, options = {}) {
  await page.goto(url, {
    waitUntil: 'networkidle2',
    timeout: 30000, // 30 second timeout
    ...options,
  });
}

4. Error in processStory Should Continue, Not Throw (Medium severity)

Location: clients/storybook/src/index.js:47-51

} catch (error) {
  logger?.error?.(
    `  ✗ Failed to capture ${story.title}/${story.name}@${viewport.name}: ${error.message}`
  );
  throw error; // ← This stops all processing
}

Issue: If one viewport fails, it stops processing other viewports for that story AND stops processing subsequent stories (due to pMap).

Recommendation: Log error but continue:

} catch (error) {
  logger?.error?.(
    `  ✗ Failed to capture ${story.title}/${story.name}@${viewport.name}: ${error.message}`
  );
  // Don't throw - continue with other viewports
}

Then track failures and report summary at the end.

5. Missing Dependency on cosmiconfig (Critical)

Location: clients/storybook/package.json

Issue: cosmiconfig is imported in src/config.js:6 but not listed in dependencies or devDependencies.

Recommendation:

npm install --save cosmiconfig

And add to package.json:

"dependencies": {
  "cosmiconfig": "^9.0.0",
  "p-map": "^8.0.1",
  "puppeteer": "^24.5.0"
}

6. CI Workflow: Missing Puppeteer Dependencies

Location: .github/workflows/ci.yml:258

Puppeteer requires system dependencies (Chrome, fonts, etc.) on Ubuntu runners.

Recommendation: Add before running tests:

- name: Install Puppeteer dependencies
  run: |
    sudo apt-get update
    sudo apt-get install -y \
      libnss3 libatk-bridge2.0-0 libx11-xcb1 libxcomposite1 \
      libxdamage1 libxrandr2 libgbm1 libpangocairo-1.0-0 \
      libgtk-3-0 libasound2

- name: Run tests
  working-directory: ./clients/storybook
  run: npm test

7. Release Workflow: Hardcoded Comparison URL

Location: .github/workflows/release-storybook-client.yml:111

**Full Changelog**: https://github.com/vizzly-testing/vizzly-cli/compare/...

Issue: Should use vizzly-testing/cli (per CLAUDE.md).

Recommendation: Update to:

**Full Changelog**: https://github.com/vizzly-testing/cli/compare/storybook/v${{ steps.current_version.outputs.version }}...storybook/v${{ steps.new_version.outputs.version }}

---

🔍 Security Review

✅ Good Practices

• No credential handling
• No eval/Function constructors
• Proper path handling with resolve()
• Browser args use safe defaults (--no-sandbox for Docker)

⚠️ Minor Concerns

1. User-provided hooks execute arbitrary code - This is by design, but ensure docs warn users about hook security
2. File:// protocol URLs - Safe for local Storybook builds, but ensure this doesn't work with remote URLs unexpectedly

---

📊 Performance Considerations

✅ Good

• Concurrency control with p-map: Prevents resource exhaustion
• Single browser instance: Reuses browser across stories (efficient)
• Page cleanup: Closes pages after each story to free memory

💡 Potential Optimization

Consider browser context pooling instead of creating new pages:

// In browser.js
export async function createBrowserContext(browser) {
  return await browser.createBrowserContext();
}

This could speed up navigation and reduce memory overhead, but current approach is fine for v1.

---

📝 Documentation

✅ Excellent

• Comprehensive README with examples
• Clear example files (vizzly-storybook.config.js, story-with-vizzly-config.stories.js)
• JSDoc comments on all functions

💡 Suggestions

1. Add troubleshooting section for common Puppeteer issues
2. Document memory usage considerations for large Storybook instances
3. Add migration guide from other visual testing tools

---

🧪 Test Coverage

✅ Strong Coverage

• Config parsing and merging ✓
• Story discovery (v6/v7) ✓
• Pattern matching ✓
• Viewport parsing ✓

🔍 Missing Tests

1. Integration test: End-to-end test with real Storybook build (mentioned in PR description as TODO)
2. Browser error handling: What happens if Puppeteer fails to launch?
3. Screenshot capture failures: Mock vizzlyScreenshot failures

Recommendation: Add integration test before first npm release:

// tests/integration/storybook-e2e.spec.js
describe('Storybook E2E', () => {
  it('should capture screenshots from real Storybook', async () => {
    // Use fixture Storybook build
    await run('./tests/fixtures/sample-storybook', {
      viewports: 'mobile:375x667',
    });
    // Verify vizzlyScreenshot was called
  });
});

---

🎯 Summary

Category	Rating	Notes
Code Quality	⭐⭐⭐⭐⭐	Excellent functional design, clean architecture
Security	⭐⭐⭐⭐	Minor path validation improvements recommended
Performance	⭐⭐⭐⭐	Good concurrency control, efficient resource usage
Test Coverage	⭐⭐⭐⭐	Strong unit tests, needs E2E test before release
Documentation	⭐⭐⭐⭐⭐	Comprehensive and well-written

---

🚀 Recommendation: Approve with Minor Changes

Before Merge (Optional but Recommended):

1. Fix missing cosmiconfig dependency (critical)
2. Add Puppeteer system dependencies to CI workflow
3. Fix comparison URL in release workflow

Before First Release:

1. Add integration test with real Storybook
2. Manual testing with various Storybook versions
3. Consider error handling improvements (continue on failure)

---

🎉 Closing Thoughts

This is a solid first plugin for Vizzly. The functional architecture is a pleasure to review, test coverage is strong, and the documentation is thorough. The issues identified are minor and don't block merging. Great work! 🚀

Congratulations on shipping the first official Vizzly plugin! 🎊