Maintenance: Extract createPreviewAnnotations factory to reduce dupli…#34832
Maintenance: Extract createPreviewAnnotations factory to reduce dupli…#34832Programer1804 wants to merge 1 commit into
Conversation
…cation across renderer presets
📝 WalkthroughWalkthroughA new shared ChangesPreview Annotations Utility Extraction
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing Touches⚔️ Resolve merge conflicts
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/core/src/common/utils/create-preview-annotations.test.ts`:
- Line 3: Update the relative import of the test helper to include an explicit
TypeScript extension: change the import of createPreviewAnnotations to use
'./create-preview-annotations.ts' so the test module explicitly references the
TypeScript file (locate the import statement that currently reads import {
createPreviewAnnotations } from './create-preview-annotations' and add the .ts
extension).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: be007e53-21ab-43c8-b78b-c6f30d797116
📒 Files selected for processing (8)
code/core/src/common/index.tscode/core/src/common/utils/create-preview-annotations.test.tscode/core/src/common/utils/create-preview-annotations.tscode/renderers/html/src/preset.tscode/renderers/preact/src/preset.tscode/renderers/svelte/src/preset.tscode/renderers/vue3/src/preset.tscode/renderers/web-components/src/preset.ts
| @@ -0,0 +1,56 @@ | |||
| import { afterEach, describe, expect, it, vi } from 'vitest'; | |||
|
|
|||
| import { createPreviewAnnotations } from './create-preview-annotations'; | |||
There was a problem hiding this comment.
Use an explicit extension in the relative test import.
Line 3 should include .ts on the local module import.
Proposed fix
-import { createPreviewAnnotations } from './create-preview-annotations';
+import { createPreviewAnnotations } from './create-preview-annotations.ts';As per coding guidelines, "For TypeScript source in the repo, prefer explicit file extensions for relative code imports and exports such as ./foo.ts or ./bar.tsx when the target is another TS/JS module; keep framework-specific component imports like .vue and .svelte in their expected form".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@code/core/src/common/utils/create-preview-annotations.test.ts` at line 3,
Update the relative import of the test helper to include an explicit TypeScript
extension: change the import of createPreviewAnnotations to use
'./create-preview-annotations.ts' so the test module explicitly references the
TypeScript file (locate the import statement that currently reads import {
createPreviewAnnotations } from './create-preview-annotations' and add the .ts
extension).
|
Unfortunately, this refactoring will cause issues in package managers with strict dependency resolution. Some package managers like pnpm will complain because they will try to e.g. resolve @storybook/react from the storybook package, which doesn't have @storybook/react defined as an entry point. Therefore, the import.meta.resolve calls has to remain in their renderer packages. |
Closes #34389
What I did
Extracted the duplicated
previewAnnotationslogic into a sharedcreatePreviewAnnotationsfactory atcode/core/src/common/utils/create-preview-annotations.tsand refactored 5 renderer presets to use it.The
previewAnnotationsfunction was nearly identical acrosshtml,preact,svelte,vue3, andweb-componentsrenderer presets, differing only in the package name string. This refactor centralizes the logic into a single factory helper, making future changes to thedocsEnabledcheck or entry resolution apply automatically to all renderers.The
reactrenderer was intentionally left out as it contains additional RSC feature-flag logic that goes beyond the scope of this change.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
No manual testing is required. This is a pure refactor with no behavior changes — the same entry files are resolved in the same order as before. The shared logic is covered by the new unit tests in
create-preview-annotations.test.ts.Summary by CodeRabbit
Release Notes
Refactor
Tests