Skip to content

fix(web): add cjs dirname for storybook#260

Merged
Brentlok merged 1 commit into
uni-stack:mainfrom
burakgormek:fix-storybook-9
Dec 24, 2025
Merged

fix(web): add cjs dirname for storybook#260
Brentlok merged 1 commit into
uni-stack:mainfrom
burakgormek:fix-storybook-9

Conversation

@burakgormek

@burakgormek burakgormek commented Dec 24, 2025

Copy link
Copy Markdown
Contributor

It looks like storybook 9 loads configs in CJS, while Storybook 10 and later use ESM.
storybookjs/storybook#31874

Fixes: #259

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced compatibility for path resolution across different module environments.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 24, 2025

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The change adds a compatibility fallback for dirname in the Vite plugin. It introduces a constant that resolves to __dirname when available, otherwise uses import.meta.dirname, fixing a Storybook 9 incompatibility where the latter is undefined.

Changes

Cohort / File(s) Summary
Vite Plugin Compatibility
packages/uniwind/src/vite/vite.ts
Added dirname compatibility alias using a ternary operator to fall back to __dirname when import.meta.dirname is unavailable, fixing path resolution failures in Storybook 9 environments

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A fallback path, so tried and true,
When dirname hides, we know what to do—
__dirname steps in with a helpful hop,
Storybook 9 works, no more stop!
✨ Compatibility magic, small but sweet.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly indicates the fix addresses CJS dirname compatibility for Storybook, directly matching the core change that adds dirname fallback logic.
Linked Issues check ✅ Passed The PR successfully addresses issue #259 by adding __dirname fallback for CJS compatibility, ensuring path.resolve receives a valid base path instead of undefined when Storybook 9 loads configs as CJS.
Out of Scope Changes check ✅ Passed The changes are narrowly scoped to the vite.ts file, adding only the dirname compatibility alias required to fix Storybook 9 CJS path resolution without introducing unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@burakgormek burakgormek marked this pull request as ready for review December 24, 2025 11:29

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/uniwind/src/vite/vite.ts (1)

17-17: Consider adding a comment explaining the CJS/ESM compatibility layer.

To improve code maintainability, add a brief comment explaining why this compatibility check exists and reference issue #259.

🔎 Suggested comment
+// CJS/ESM compatibility: Storybook 9 loads configs as CJS where import.meta.dirname is undefined.
+// Fall back to __dirname in CJS mode. See issue #259.
 const dirname = typeof __dirname !== 'undefined' ? __dirname : import.meta.dirname
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 634fd60 and ccf7644.

📒 Files selected for processing (1)
  • packages/uniwind/src/vite/vite.ts

Comment on lines +17 to 21
const dirname = typeof __dirname !== 'undefined' ? __dirname : import.meta.dirname
const componentPath = path.resolve(
import.meta.dirname,
dirname,
'../module/components/web/index.js',
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check Node.js engine requirements in package.json
echo "=== Checking Node.js engine requirements ==="
if [ -f "package.json" ]; then
    jq '.engines.node // "Not specified"' package.json
fi

# Also check monorepo root and specific package
echo -e "\n=== Checking packages/uniwind/package.json ==="
if [ -f "packages/uniwind/package.json" ]; then
    jq '.engines.node // "Not specified"' packages/uniwind/package.json
fi

# Check for other uses of import.meta.dirname in the codebase
echo -e "\n=== Checking for other import.meta.dirname usage ==="
rg -n 'import\.meta\.dirname' --type ts --type js 2>/dev/null | head -20

# Check the actual file to see the full context
echo -e "\n=== Full context of packages/uniwind/src/vite/vite.ts ==="
if [ -f "packages/uniwind/src/vite/vite.ts" ]; then
    wc -l packages/uniwind/src/vite/vite.ts
fi

Repository: uni-stack/uniwind

Length of output: 574


🏁 Script executed:

#!/bin/bash
# Check for fileURLToPath usage in the codebase to see if it's already imported/used
echo "=== Checking for fileURLToPath usage ==="
rg -n 'fileURLToPath' --type ts --type js 2>/dev/null

# Check if there are any Node.js version constraints mentioned
echo -e "\n=== Checking Vite and related dependency versions ==="
if [ -f "package.json" ]; then
    jq '.dependencies.vite, .devDependencies.vite // "Not found"' package.json
fi

# Check the actual diff/change to understand what was modified
echo -e "\n=== Checking git history for recent changes to this file ==="
git log --oneline -n 10 -- packages/uniwind/src/vite/vite.ts 2>/dev/null || echo "Git history not available"

Repository: uni-stack/uniwind

Length of output: 283


🏁 Script executed:

#!/bin/bash
# Get full content of the file to verify the fix
cat -n packages/uniwind/src/vite/vite.ts | head -30

Repository: uni-stack/uniwind

Length of output: 1156


Use import.meta.url for broader Node.js compatibility and address duplicate pattern in css/index.ts.

While the CJS/ESM detection is correct for Storybook 9, import.meta.dirname requires Node.js 20.11.0+. Since no Node.js version is constrained in package.json, this introduces a portability risk: ESM code on older Node.js would get undefined and fail with the same error. The same pattern exists in packages/uniwind/src/css/index.ts:7.

Use import.meta.url instead:

import { fileURLToPath } from 'url'
-const dirname = typeof __dirname !== 'undefined' ? __dirname : import.meta.dirname
+const dirname = typeof __dirname !== 'undefined' 
+    ? __dirname 
+    : path.dirname(fileURLToPath(import.meta.url))

This works across all Node.js versions supporting ESM. Apply the same fix to packages/uniwind/src/css/index.ts.

Additionally, verify this resolves the Storybook 9 issue by testing with the reproduction from issue #259.

🤖 Prompt for AI Agents
In packages/uniwind/src/vite/vite.ts around lines 17 to 21, replace the current
__dirname / import.meta.dirname detection with a robust ESM-compatible approach
using fileURLToPath(import.meta.url): import fileURLToPath from 'url', compute
dirname = typeof __dirname !== 'undefined' ? __dirname :
path.dirname(fileURLToPath(import.meta.url)), then use that dirname in
path.resolve to build componentPath; apply the same change to
packages/uniwind/src/css/index.ts at the same pattern (line ~7) so both modules
use fileURLToPath(import.meta.url) fallback for broader Node.js compatibility
and then run the reproduction from issue #259 to verify Storybook 9 is resolved.

@Brentlok Brentlok merged commit 555f3b7 into uni-stack:main Dec 24, 2025
2 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.

Vite plugin doesn't work with Storybook 9

2 participants