Addon-Vitest: Support non-ASCII project paths#34686
Conversation
📝 WalkthroughWalkthroughThe PR adds URI decoding support to the Vitest plugin to handle non-ASCII project paths and URL-encoded story module IDs. A new ChangesURI Decoding in Vitest Plugin
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/addons/vitest/src/vitest-plugin/index.test.ts (1)
3-3: 💤 Low value
vitestTransformmock should return aPromise.Per coding guidelines, mock implementations for async functions must return a
Promise. The test works correctly today because the surroundingasync transformhandler wraps the synchronous return, but aligning with the codebase standard avoids confusion.♻️ Proposed fix
-const vitestTransform = vi.hoisted(() => vi.fn(() => ({ code: 'transformed code' }))); +const vitestTransform = vi.hoisted(() => vi.fn(async () => ({ code: 'transformed code' })));As per coding guidelines, "Each mock implementation should return a Promise for async functions in Vitest tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/vitest/src/vitest-plugin/index.test.ts` at line 3, The vitestTransform mock (created via vi.hoisted and vi.fn) is returning a plain object but should return a Promise to match async function conventions; update the mock implementation used in vitest-plugin/index.test.ts so vi.fn(() => ({ code: 'transformed code' })) becomes an implementation that returns Promise.resolve({ code: 'transformed code' }) (i.e., ensure vitestTransform resolves a Promise) so the mock semantics align with the async transform handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@code/addons/vitest/src/vitest-plugin/index.test.ts`:
- Line 3: The vitestTransform mock (created via vi.hoisted and vi.fn) is
returning a plain object but should return a Promise to match async function
conventions; update the mock implementation used in vitest-plugin/index.test.ts
so vi.fn(() => ({ code: 'transformed code' })) becomes an implementation that
returns Promise.resolve({ code: 'transformed code' }) (i.e., ensure
vitestTransform resolves a Promise) so the mock semantics align with the async
transform handler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e22ebed-1e9e-46ee-a58b-3bcdd5e5357d
📒 Files selected for processing (2)
code/addons/vitest/src/vitest-plugin/index.test.tscode/addons/vitest/src/vitest-plugin/index.ts
|
Hi @cyphercodes, Thanks for tackling this! Two things worth considering as follow-ups:
|
Closes #33700
What I did
téststill run story files through the Storybook Vitest transform.AI assistance: OpenAI Codex via Hermes Agent helped create this contribution.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
No browser manual testing was needed for this path-matching fix. I ran:
yarn exec oxfmt --check code/addons/vitest/src/vitest-plugin/index.ts code/addons/vitest/src/vitest-plugin/index.test.tsNODE_OPTIONS=--max-old-space-size=4096 yarn --cwd code lint:js:cmd addons/vitest/src/vitest-plugin/index.ts addons/vitest/src/vitest-plugin/index.test.tsyarn vitest run --config code/addons/vitest/vitest.config.ts code/addons/vitest/srcyarn nx check addon-vitestDocumentation
Summary by CodeRabbit