Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions code/addons/vitest/src/vitest-plugin/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,10 @@ import { type RunnerTask, type TaskMeta, type TestContext } from 'vitest';
import { type Meta, type Story, getStoryChildren, isStory } from 'storybook/internal/csf';
import type { ComponentAnnotations, ComposedStoryFn, Renderer } from 'storybook/internal/types';

import { server } from '@vitest/browser/context';
import { type Report, composeStory, getCsfFactoryAnnotations } from 'storybook/preview-api';

import { setViewport } from './viewports';

declare module '@vitest/browser/context' {
interface BrowserCommands {
getInitialGlobals: () => Promise<Record<string, any>>;
}
}

const { getInitialGlobals } = server.commands;
import { getVitestBrowserContext } from './vitest-context';

/**
* Converts a file URL to a file path, handling URL encoding
Expand Down Expand Up @@ -49,6 +41,10 @@ export const testStory = (

const storyAnnotations = test ? test.input : annotations.story;

const { server } = await getVitestBrowserContext();

const { getInitialGlobals } = server.commands;

const composedStory = composeStory(
storyAnnotations,
annotations.meta!,
Expand Down
4 changes: 3 additions & 1 deletion code/addons/vitest/src/vitest-plugin/viewports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { UnsupportedViewportDimensionError } from 'storybook/internal/preview-er
import { MINIMAL_VIEWPORTS } from 'storybook/viewport';
import type { ViewportMap } from 'storybook/viewport';

import { getVitestBrowserContext } from './vitest-context';

declare global {
// eslint-disable-next-line no-var
var __vitest_browser__: boolean;
Expand Down Expand Up @@ -66,7 +68,7 @@ export const setViewport = async (parameters: Parameters = {}, globals: Globals
defaultViewport = viewportsParam.defaultViewport;
}

const { page } = await import('@vitest/browser/context').catch(() => ({
const { page } = await getVitestBrowserContext().catch(() => ({
page: null,
}));

Expand Down
24 changes: 24 additions & 0 deletions code/addons/vitest/src/vitest-plugin/vitest-context.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import semver from 'semver';
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Replace version sniffing with feature detection to improve robustness.

The current approach uses JSON import attributes and semver to detect Vitest version, but this has several drawbacks:

  1. Import attributes brittleness: JSON import attributes (with: { type: 'json' }) are not universally supported across all Node versions and bundlers, and the module shape may vary (direct properties vs default.version).
  2. Runtime overhead: Adding semver as a runtime dependency just for version detection is heavy.
  3. Maintenance burden: Version checks require updates when Vitest changes its versioning scheme.

A previous review comment suggested using feature detection instead, which is more robust and eliminates the semver dependency.

Apply the feature detection approach suggested in the prior review:

-import semver from 'semver';
-
 type VitestBrowserContext = typeof import('@vitest/browser/context');
 
 type VitestServerContext = VitestBrowserContext & {
   server: {
     commands: typeof import('@vitest/browser/context').server.commands & {
       getInitialGlobals: () => Promise<Record<string, any>>;
     };
   };
 };
 
-/** Gets the Vitest browser context based on which version of Vitest is installed. */
+/** Gets the Vitest browser context by feature detection (v4 first, then v3). */
 export const getVitestBrowserContext = async (): Promise<VitestServerContext> => {
-  const vitestVersion = await import('vitest/package.json', { with: { type: 'json' } }).then(
-    (v) => v.version
-  );
-
-  if (semver.major(vitestVersion) >= 4) {
-    return import('vitest/browser') as unknown as Promise<VitestServerContext>;
+  try {
+    return (await import('vitest/browser')) as unknown as VitestServerContext;
+  } catch {
+    return (await import('@vitest/browser/context')) as unknown as VitestServerContext;
   }
-
-  return import('@vitest/browser/context') as Promise<VitestServerContext>;
 };

Optionally, add caching to avoid repeated dynamic imports:

let _ctxPromise: Promise<VitestServerContext> | undefined;

export const getVitestBrowserContext = async (): Promise<VitestServerContext> => {
  if (_ctxPromise) return _ctxPromise;
  _ctxPromise = (async () => {
    try {
      return (await import('vitest/browser')) as unknown as VitestServerContext;
    } catch {
      return (await import('@vitest/browser/context')) as unknown as VitestServerContext;
    }
  })();
  return _ctxPromise;
};

Also applies to: 15-20

🤖 Prompt for AI Agents
In code/addons/vitest/src/vitest-plugin/vitest-context.ts around lines 1 and
15-20, replace the current Vitest version-sniffing logic (including the semver
dependency and JSON import-attributes) with feature-detection via attempting to
import the newer module path first and falling back to the older one; implement
an async cached loader (a top-level Promise variable) so repeated calls reuse
the same dynamic import result; remove the semver import and any JSON-with-type
import logic and update exports to use the resolved module shape directly as the
VitestServerContext.


type VitestBrowserContext = typeof import('@vitest/browser/context');

type VitestServerContext = VitestBrowserContext & {
server: {
commands: typeof import('@vitest/browser/context').server.commands & {
getInitialGlobals: () => Promise<Record<string, any>>;
};
};
};

/** Gets the Vitest browser context based on which version of Vitest is installed. */
export const getVitestBrowserContext = async (): Promise<VitestServerContext> => {
const vitestVersion = await import('vitest/package.json', { with: { type: 'json' } }).then(
(v) => v.version
);

if (semver.major(vitestVersion) >= 4) {
return import('@vitest/browser') as unknown as Promise<VitestServerContext>;
}

return import('@vitest/browser/context') as Promise<VitestServerContext>;
};
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,15 @@ export const viteInjectMockerRuntime = (options: {
config() {
return {
optimizeDeps: {
include: ['@vitest/mocker', '@vitest/mocker/browser'],
include: ['@vitest/mocker', '@vitest/mocker/browser', '@vitest/mocker/node'],
exclude: ['fsevents'],
},
resolve: {
// Aliasing necessary for package managers like pnpm, since resolving modules from a virtual module
// leads to errors, if the imported module is not a dependency of the project.
// By resolving the module to the real path, we can avoid this issue.
alias: {
'@vitest/mocker/node': fileURLToPath(import.meta.resolve('@vitest/mocker/node')),
'@vitest/mocker/browser': fileURLToPath(import.meta.resolve('@vitest/mocker/browser')),
'@vitest/mocker': fileURLToPath(import.meta.resolve('@vitest/mocker')),
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import type { Preview } from '@storybook/react-vite';
import { sb } from 'storybook/test'
import { getDecoratorString } from './get-decorator-string';

sb.mock('./get-decorator-string.ts', { spy: true });

console.log('preview file is called!');

const preview: Preview = {
Expand Down
15 changes: 7 additions & 8 deletions test-storybooks/portable-stories-kitchen-sink/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@
"devDependencies": {
"@playwright/experimental-ct-react": "1.52.0",
"@playwright/test": "1.52.0",
"@storybook/addon-a11y": "^8.0.0",
"@storybook/addon-vitest": "^8.0.0",
"@storybook/react": "^8.0.0",
"@storybook/react-vite": "^8.0.0",
"@storybook/addon-a11y": "^10.0.0",
"@storybook/addon-vitest": "^10.0.0",
"@storybook/react": "^10.0.0",
"@storybook/react-vite": "^10.0.0",
"@swc/core": "^1.4.2",
"@swc/jest": "^0.2.36",
"@testing-library/dom": "^10.4.1",
Expand All @@ -74,8 +74,7 @@
"@types/react-dom": "^19.0.3",
"@typescript-eslint/eslint-plugin": "^6.21.0",
"@typescript-eslint/parser": "^6.21.0",
"@vitejs/plugin-react": "^4.2.1",
"@vitest/browser": "^4.0.0",
"@vitejs/plugin-react": "^5.0.4",
"@vitest/browser-playwright": "^4.0.0",
"@vitest/coverage-v8": "^4.0.0",
"@vitest/ui": "^4.0.0",
Expand All @@ -87,9 +86,9 @@
"identity-obj-proxy": "^3.0.0",
"jest": "^29.7.0",
"jest-environment-jsdom": "^29.7.0",
"storybook": "^8.0.0",
"storybook": "^10.0.0",
"typescript": "^5.8.3",
"vite": "^5.1.1",
"vite": "^7.1.12",
"vitest": "^4.0.0"
}
}