From 2cde356c7807eb5b89e81fc89b91bf49546e380b Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Wed, 23 Oct 2024 16:22:07 +0200 Subject: [PATCH 1/6] Revert "Revert "Addon Test: Error when addon interactions exists"" --- code/.storybook/main.ts | 1 - code/addons/interactions/src/preset.ts | 3 +++ code/addons/test/src/preset.ts | 33 +++++++++++++++++++++++++- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/code/.storybook/main.ts b/code/.storybook/main.ts index 513fde263a2..ab8af9af8f4 100644 --- a/code/.storybook/main.ts +++ b/code/.storybook/main.ts @@ -96,7 +96,6 @@ const config: StorybookConfig = { addons: [ '@storybook/addon-themes', '@storybook/addon-essentials', - '@storybook/addon-interactions', '@storybook/addon-storysource', '@storybook/addon-designs', '@storybook/experimental-addon-test', diff --git a/code/addons/interactions/src/preset.ts b/code/addons/interactions/src/preset.ts index b58832d5e36..2e3dae62404 100644 --- a/code/addons/interactions/src/preset.ts +++ b/code/addons/interactions/src/preset.ts @@ -18,3 +18,6 @@ export const checkActionsLoaded = (configDir: string) => { getConfig: (configFile) => serverRequire(configFile), }); }; + +// This annotation is read by addon-test, so it can throw an error if both addons are used +export const ADDON_INTERACTIONS_IN_USE = true; diff --git a/code/addons/test/src/preset.ts b/code/addons/test/src/preset.ts index dc8bbfd88b2..1e0ba3a16b7 100644 --- a/code/addons/test/src/preset.ts +++ b/code/addons/test/src/preset.ts @@ -9,7 +9,7 @@ import { TESTING_MODULE_WATCH_MODE_REQUEST, } from 'storybook/internal/core-events'; import { oneWayHash, telemetry } from 'storybook/internal/telemetry'; -import type { Options, StoryId } from 'storybook/internal/types'; +import type { Options, PresetProperty, StoryId } from 'storybook/internal/types'; import picocolors from 'picocolors'; import { dedent } from 'ts-dedent'; @@ -99,3 +99,34 @@ export const experimental_serverChannel = async (channel: Channel, options: Opti return channel; }; + +export const previewAnnotations: PresetProperty<'previewAnnotations'> = async ( + entry = [], + options +) => { + checkActionsLoaded(options.configDir); + return entry; +}; + +export const managerEntries: PresetProperty<'managerEntries'> = async (entry = [], options) => { + // Throw an error when addon-interactions is used. + // This is done by reading an annotation defined in addon-interactions, which although not ideal, + // is a way to handle addon conflict without having to worry about the order of which they are registered + const annotation = await options.presets.apply('ADDON_INTERACTIONS_IN_USE', false); + if (annotation) { + // eslint-disable-next-line local-rules/no-uncategorized-errors + const error = new Error( + dedent` + You have both addon-interactions and addon-test enabled, which is not allowed. + + addon-test is a replacement for addon-interactions, please uninstall and remove addon-interactions from the addons list in your main config at ${options.configDir}. + ` + ); + error.name = 'AddonConflictError'; + + throw error; + } + + // for whatever reason seems like the return type of managerEntries is not correct (it expects never instead of string[]) + return entry as never; +}; From f7c632d2097d1c5dd965245eac3d0fe020f8ce3e Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Thu, 24 Oct 2024 14:22:37 +0200 Subject: [PATCH 2/6] Setup @storybook/experimental-addon-test just for authorized sandboxes --- scripts/task.ts | 2 +- scripts/tasks/sandbox-parts.ts | 13 ++------ scripts/tasks/sandbox.ts | 56 +++++++++++++++++++++------------- scripts/utils/cli-step.ts | 4 ++- 4 files changed, 40 insertions(+), 35 deletions(-) diff --git a/scripts/task.ts b/scripts/task.ts index 9a28f8e7954..69c949254b3 100644 --- a/scripts/task.ts +++ b/scripts/task.ts @@ -38,7 +38,7 @@ import { createOptions, getCommand, getOptionsOrPrompt } from './utils/options'; const sandboxDir = process.env.SANDBOX_ROOT || SANDBOX_DIRECTORY; -export const extraAddons = ['a11y', 'storysource']; +export const extraAddons = ['@storybook/addon-a11y', '@storybook/addon-storysource']; export type Path = string; export type TemplateDetails = { diff --git a/scripts/tasks/sandbox-parts.ts b/scripts/tasks/sandbox-parts.ts index c090cc69e5b..d785be75aa5 100644 --- a/scripts/tasks/sandbox-parts.ts +++ b/scripts/tasks/sandbox-parts.ts @@ -24,7 +24,6 @@ import { detectLanguage } from '../../code/core/src/cli/detect'; import { SupportedLanguage } from '../../code/core/src/cli/project_types'; import { JsPackageManagerFactory, - removeAddon, versions as storybookPackages, } from '../../code/core/src/common'; import type { ConfigFile } from '../../code/core/src/csf-tools'; @@ -181,8 +180,7 @@ export const init: Task['run'] = async ( if (!skipTemplateStories) { for (const addon of addons) { - const addonName = `@storybook/addon-${addon}`; - await executeCLIStep(steps.add, { argument: addonName, cwd, dryRun, debug }); + await executeCLIStep(steps.add, { argument: addon, cwd, dryRun, debug, optionValues: { yes: true } }); } } }; @@ -395,13 +393,6 @@ const getVitestPluginInfo = (details: TemplateDetails) => { export async function setupVitest(details: TemplateDetails, options: PassedOptionValues) { const { sandboxDir, template } = details; - // Remove interactions addon to avoid issues with Vitest - // TODO: add an if statement when we introduce a sandbox that tests interactions - await removeAddon('@storybook/addon-interactions', { - cwd: details.sandboxDir, - configDir: join(details.sandboxDir, '.storybook'), - }); - const packageJsonPath = join(sandboxDir, 'package.json'); const packageJson = await readJson(packageJsonPath); @@ -804,7 +795,7 @@ export const extendMain: Task['run'] = async ({ template, sandboxDir, key }, { d } const addons = mainConfig.getFieldValue(['addons']); - mainConfig.setFieldValue(['addons'], [...addons, '@storybook/experimental-addon-test']); + mainConfig.setFieldValue(['addons'], [...addons]); if (template.expected.builder === '@storybook/builder-vite') { setSandboxViteFinal(mainConfig); diff --git a/scripts/tasks/sandbox.ts b/scripts/tasks/sandbox.ts index 9264c31fa13..66887f20553 100644 --- a/scripts/tasks/sandbox.ts +++ b/scripts/tasks/sandbox.ts @@ -50,6 +50,39 @@ export const sandbox: Task = { setupVitest, } = await import('./sandbox-parts'); + const extraDeps = [ + ...(details.template.modifications?.extraDependencies ?? []), + // The storybook package forwards some CLI commands to @storybook/cli with npx. + // Adding the dep makes sure that even npx will use the linked workspace version. + '@storybook/cli', + ]; + + const shouldAddVitestIntegration = !details.template.skipTasks?.includes('vitest-integration') + + if (shouldAddVitestIntegration) { + extraDeps.push( + 'happy-dom', + 'vitest', + 'playwright', + '@vitest/browser', + ); + + if (details.template.expected.framework.includes('nextjs')) { + extraDeps.push('@storybook/experimental-nextjs-vite', 'jsdom'); + } + + // if (details.template.expected.renderer === '@storybook/svelte') { + // extraDeps.push(`@testing-library/svelte`); + // } + // + // if (details.template.expected.framework === '@storybook/angular') { + // extraDeps.push('@testing-library/angular', '@analogjs/vitest-angular'); + // } + + options.addon = [...options.addon, '@storybook/experimental-addon-test']; + } + + let startTime = now(); await create(details, options); const createTime = now() - startTime; @@ -83,28 +116,7 @@ export const sandbox: Task = { await addStories(details, options); } - const extraDeps = [ - ...(details.template.modifications?.extraDependencies ?? []), - // The storybook package forwards some CLI commands to @storybook/cli with npx. - // Adding the dep makes sure that even npx will use the linked workspace version. - '@storybook/cli', - '@storybook/experimental-addon-test', - ]; - if (!details.template.skipTasks?.includes('vitest-integration')) { - extraDeps.push('happy-dom', 'vitest', 'playwright', '@vitest/browser'); - - if (details.template.expected.framework.includes('nextjs')) { - extraDeps.push('@storybook/experimental-nextjs-vite', 'jsdom'); - } - - // if (details.template.expected.renderer === '@storybook/svelte') { - // extraDeps.push(`@testing-library/svelte`); - // } - // - // if (details.template.expected.framework === '@storybook/angular') { - // extraDeps.push('@testing-library/angular', '@analogjs/vitest-angular'); - // } - + if(shouldAddVitestIntegration) { await setupVitest(details, options); } diff --git a/scripts/utils/cli-step.ts b/scripts/utils/cli-step.ts index b428160490f..ce57896aedd 100644 --- a/scripts/utils/cli-step.ts +++ b/scripts/utils/cli-step.ts @@ -47,7 +47,9 @@ export const steps = { description: 'Adding addon', icon: '+', hasArgument: true, - options: createOptions({}), + options: createOptions({ + yes: { type: 'boolean' }, + }), }, link: { command: 'link', From 6be3da3666a80bec4ee83f8f18b934fd30f5e369 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 24 Oct 2024 17:06:56 +0200 Subject: [PATCH 3/6] Rewordings --- code/addons/test/src/preset.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/code/addons/test/src/preset.ts b/code/addons/test/src/preset.ts index 1e0ba3a16b7..1d4203a3d5a 100644 --- a/code/addons/test/src/preset.ts +++ b/code/addons/test/src/preset.ts @@ -117,13 +117,12 @@ export const managerEntries: PresetProperty<'managerEntries'> = async (entry = [ // eslint-disable-next-line local-rules/no-uncategorized-errors const error = new Error( dedent` - You have both addon-interactions and addon-test enabled, which is not allowed. + You have both "@storybook/addon-interactions" and "@storybook/experimental-addon-test" listed as addons in your Storybook config. This is not allowed, as @storybook/experimental-addon-test is a replacement for @storybook/addon-interactions. - addon-test is a replacement for addon-interactions, please uninstall and remove addon-interactions from the addons list in your main config at ${options.configDir}. + Please remove "@storybook/addon-interactions" from the addons array in your main Storybook config at ${options.configDir} and remove the dependency from your package.json file. ` ); error.name = 'AddonConflictError'; - throw error; } From 018cb2fcda344afea783f826b2efc78075b4ffce Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 24 Oct 2024 17:11:35 +0200 Subject: [PATCH 4/6] Remove unnecessary re-setting of field value, and apply linting fixes --- scripts/tasks/sandbox-parts.ts | 16 ++++++++-------- scripts/tasks/sandbox.ts | 12 +++--------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/scripts/tasks/sandbox-parts.ts b/scripts/tasks/sandbox-parts.ts index d785be75aa5..7123357ae93 100644 --- a/scripts/tasks/sandbox-parts.ts +++ b/scripts/tasks/sandbox-parts.ts @@ -22,10 +22,7 @@ import dedent from 'ts-dedent'; import { babelParse } from '../../code/core/src/babel/babelParse'; import { detectLanguage } from '../../code/core/src/cli/detect'; import { SupportedLanguage } from '../../code/core/src/cli/project_types'; -import { - JsPackageManagerFactory, - versions as storybookPackages, -} from '../../code/core/src/common'; +import { JsPackageManagerFactory, versions as storybookPackages } from '../../code/core/src/common'; import type { ConfigFile } from '../../code/core/src/csf-tools'; import { writeConfig } from '../../code/core/src/csf-tools'; import type { TemplateKey } from '../../code/lib/cli-storybook/src/sandbox-templates'; @@ -180,7 +177,13 @@ export const init: Task['run'] = async ( if (!skipTemplateStories) { for (const addon of addons) { - await executeCLIStep(steps.add, { argument: addon, cwd, dryRun, debug, optionValues: { yes: true } }); + await executeCLIStep(steps.add, { + argument: addon, + cwd, + dryRun, + debug, + optionValues: { yes: true }, + }); } } }; @@ -794,9 +797,6 @@ export const extendMain: Task['run'] = async ({ template, sandboxDir, key }, { d mainConfig.setFieldValue(['stories'], updatedStories); } - const addons = mainConfig.getFieldValue(['addons']); - mainConfig.setFieldValue(['addons'], [...addons]); - if (template.expected.builder === '@storybook/builder-vite') { setSandboxViteFinal(mainConfig); } diff --git a/scripts/tasks/sandbox.ts b/scripts/tasks/sandbox.ts index 66887f20553..a6064b2d58e 100644 --- a/scripts/tasks/sandbox.ts +++ b/scripts/tasks/sandbox.ts @@ -57,15 +57,10 @@ export const sandbox: Task = { '@storybook/cli', ]; - const shouldAddVitestIntegration = !details.template.skipTasks?.includes('vitest-integration') + const shouldAddVitestIntegration = !details.template.skipTasks?.includes('vitest-integration'); if (shouldAddVitestIntegration) { - extraDeps.push( - 'happy-dom', - 'vitest', - 'playwright', - '@vitest/browser', - ); + extraDeps.push('happy-dom', 'vitest', 'playwright', '@vitest/browser'); if (details.template.expected.framework.includes('nextjs')) { extraDeps.push('@storybook/experimental-nextjs-vite', 'jsdom'); @@ -82,7 +77,6 @@ export const sandbox: Task = { options.addon = [...options.addon, '@storybook/experimental-addon-test']; } - let startTime = now(); await create(details, options); const createTime = now() - startTime; @@ -116,7 +110,7 @@ export const sandbox: Task = { await addStories(details, options); } - if(shouldAddVitestIntegration) { + if (shouldAddVitestIntegration) { await setupVitest(details, options); } From 3b870cc568a8293a2d379ec2af08a1bb14a8d270 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 25 Oct 2024 12:35:07 +0200 Subject: [PATCH 5/6] Fix E2E tests --- code/e2e-tests/addon-interactions.spec.ts | 6 +++--- code/e2e-tests/addon-test.spec.ts | 7 ++++++- code/e2e-tests/preview-api.spec.ts | 10 +++++++--- code/e2e-tests/util.ts | 8 ++++++++ 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/code/e2e-tests/addon-interactions.spec.ts b/code/e2e-tests/addon-interactions.spec.ts index d236e4e1e8a..b5a703e2d1e 100644 --- a/code/e2e-tests/addon-interactions.spec.ts +++ b/code/e2e-tests/addon-interactions.spec.ts @@ -1,17 +1,17 @@ import { expect, test } from '@playwright/test'; import process from 'process'; -import { SbPage } from './util'; +import { SbPage, hasVitestIntegration } from './util'; const storybookUrl = process.env.STORYBOOK_URL || 'http://localhost:8001'; const templateName = process.env.STORYBOOK_TEMPLATE_NAME || ''; test.describe('addon-interactions', () => { - // TODO: fix the skip statement below when we introduce a sandbox that tests interactions test.skip( - templateName !== 'todo-sandbox-with-addon-interactions', + hasVitestIntegration, `Skipping ${templateName}, which does not have addon-interactions set up.` ); + test.beforeEach(async ({ page }) => { await page.goto(storybookUrl); await new SbPage(page, expect).waitUntilLoaded(); diff --git a/code/e2e-tests/addon-test.spec.ts b/code/e2e-tests/addon-test.spec.ts index 67b97409f2a..cca2f088eb9 100644 --- a/code/e2e-tests/addon-test.spec.ts +++ b/code/e2e-tests/addon-test.spec.ts @@ -1,12 +1,17 @@ import { expect, test } from '@playwright/test'; import process from 'process'; -import { SbPage } from './util'; +import { SbPage, hasVitestIntegration } from './util'; const storybookUrl = process.env.STORYBOOK_URL || 'http://localhost:8001'; const templateName = process.env.STORYBOOK_TEMPLATE_NAME || ''; test.describe('addon-test', () => { + test.skip( + !hasVitestIntegration, + `Skipping ${templateName}, which does not have addon-test set up.` + ); + test.beforeEach(async ({ page }) => { await page.goto(storybookUrl); await new SbPage(page, expect).waitUntilLoaded(); diff --git a/code/e2e-tests/preview-api.spec.ts b/code/e2e-tests/preview-api.spec.ts index df93a724851..cfabaf89674 100644 --- a/code/e2e-tests/preview-api.spec.ts +++ b/code/e2e-tests/preview-api.spec.ts @@ -1,7 +1,7 @@ import { expect, test } from '@playwright/test'; import process from 'process'; -import { SbPage } from './util'; +import { SbPage, hasVitestIntegration } from './util'; const storybookUrl = process.env.STORYBOOK_URL || 'http://localhost:8001'; const templateName = process.env.STORYBOOK_TEMPLATE_NAME || ''; @@ -26,8 +26,12 @@ test.describe('preview-api', () => { await expect(sbPage.page.locator('.sidebar-container')).toBeVisible(); // wait for the play function to complete - await sbPage.viewAddonPanel('Component tests'); - const interactionsTab = page.locator('#tabbutton-storybook-test-panel'); + await sbPage.viewAddonPanel(hasVitestIntegration ? 'Component tests' : 'Interactions'); + const interactionsTab = page.locator( + hasVitestIntegration + ? '#tabbutton-storybook-test-panel' + : '#tabbutton-storybook-interactions-panel' + ); await expect(interactionsTab).toBeVisible(); const panel = sbPage.panelContent(); const runStatusBadge = panel.locator('[aria-label="Status of the test run"]'); diff --git a/code/e2e-tests/util.ts b/code/e2e-tests/util.ts index 04333e039da..057dc91f715 100644 --- a/code/e2e-tests/util.ts +++ b/code/e2e-tests/util.ts @@ -2,6 +2,8 @@ import { toId } from '@storybook/csf'; import type { Expect, Page } from '@playwright/test'; +import { allTemplates } from '../lib/cli-storybook/src/sandbox-templates'; + export class SbPage { readonly page: Page; @@ -142,3 +144,9 @@ export class SbPage { return this.previewIframe().locator('body'); } } + +const templateName: keyof typeof allTemplates = process.env.STORYBOOK_TEMPLATE_NAME || ('' as any); + +const templates = allTemplates; +export const hasVitestIntegration = + !templates[templateName]?.skipTasks?.includes('vitest-integration'); From c02fe507b471771d5fbf7cf228829ca703108b47 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 25 Oct 2024 13:49:00 +0200 Subject: [PATCH 6/6] Show notifications in sidebar even if testing-module is not in place --- code/core/src/manager/components/sidebar/SidebarBottom.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/core/src/manager/components/sidebar/SidebarBottom.tsx b/code/core/src/manager/components/sidebar/SidebarBottom.tsx index d2fd09e82e2..48bf60a5f09 100644 --- a/code/core/src/manager/components/sidebar/SidebarBottom.tsx +++ b/code/core/src/manager/components/sidebar/SidebarBottom.tsx @@ -211,7 +211,7 @@ export const SidebarBottomBase = ({ api, notifications = [], status = {} }: Side }, [api, testProviders, updateTestProvider, clearState]); const testProvidersArray = Object.values(testProviders); - if (!hasWarnings && !hasErrors && !testProvidersArray.length) { + if (!hasWarnings && !hasErrors && !testProvidersArray.length && !notifications.length) { return null; }