From 10d3efeaca74d8357161f483fd9ca8eaea3bd6f0 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 10 Jan 2024 18:32:27 +0800 Subject: [PATCH 1/7] Indexer: Add metaTags and make autodocs inherit them --- code/lib/core-server/src/presets/common-manager.ts | 7 +------ code/lib/core-server/src/utils/StoryIndexGenerator.ts | 5 +++-- code/lib/csf-tools/src/CsfFile.ts | 1 + code/lib/types/src/modules/indexer.ts | 2 ++ 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/code/lib/core-server/src/presets/common-manager.ts b/code/lib/core-server/src/presets/common-manager.ts index 11fde917d143..f8ecf896d84d 100644 --- a/code/lib/core-server/src/presets/common-manager.ts +++ b/code/lib/core-server/src/presets/common-manager.ts @@ -14,11 +14,6 @@ const excludeTags = Object.entries(global.TAGS_OPTIONS ?? {}).reduce((acc, entry addons.register(STATIC_FILTER, (api) => { api.experimental_setFilter(STATIC_FILTER, (item) => { const tags = item.tags || []; - // very strange behavior here. Auto-generated docs entries get - // the tags of the primary story by default, so if that story - // happens to be `docs-only`, then filtering it out of the sidebar - // ALSO filter out the sidebar entry, which is not what we want. - // Here we special case it, but there should be a better solution. - return tags.includes('docs') || tags.filter((tag) => excludeTags[tag]).length === 0; + return tags.filter((tag) => excludeTags[tag]).length === 0; }); }); diff --git a/code/lib/core-server/src/utils/StoryIndexGenerator.ts b/code/lib/core-server/src/utils/StoryIndexGenerator.ts index 96e72091073a..0f62104d55e0 100644 --- a/code/lib/core-server/src/utils/StoryIndexGenerator.ts +++ b/code/lib/core-server/src/utils/StoryIndexGenerator.ts @@ -319,7 +319,7 @@ export class StoryIndexGenerator { const name = this.options.docs.defaultName ?? 'Docs'; const { metaId } = indexInputs[0]; const { title } = entries[0]; - const tags = indexInputs[0].tags || []; + const metaTags = indexInputs[0].metaTags || []; const id = toId(metaId ?? title, name); entries.unshift({ id, @@ -327,7 +327,7 @@ export class StoryIndexGenerator { name, importPath, type: 'docs', - tags: [...tags, 'docs', ...(!hasAutodocsTag && !isStoriesMdx ? [AUTODOCS_TAG] : [])], + tags: [...metaTags, 'docs', ...(!hasAutodocsTag && !isStoriesMdx ? [AUTODOCS_TAG] : [])], storiesImports: [], }); } @@ -438,6 +438,7 @@ export class StoryIndexGenerator { importPath, storiesImports: sortedDependencies.map((dep) => dep.entries[0].importPath), type: 'docs', + // FIXME: update this to use the index entry's metaTags once we update this to run on `IndexInputs` tags: [...(result.tags || []), csfEntry ? 'attached-mdx' : 'unattached-mdx', 'docs'], }; return docsEntry; diff --git a/code/lib/csf-tools/src/CsfFile.ts b/code/lib/csf-tools/src/CsfFile.ts index 897f6b2e0b0d..2e4745281739 100644 --- a/code/lib/csf-tools/src/CsfFile.ts +++ b/code/lib/csf-tools/src/CsfFile.ts @@ -572,6 +572,7 @@ export class CsfFile { title: this.meta?.title, metaId: this.meta?.id, tags, + metaTags: this.meta?.tags, __id: story.id, }; }); diff --git a/code/lib/types/src/modules/indexer.ts b/code/lib/types/src/modules/indexer.ts index 56d435cdb533..1064354eefe0 100644 --- a/code/lib/types/src/modules/indexer.ts +++ b/code/lib/types/src/modules/indexer.ts @@ -107,6 +107,8 @@ export type BaseIndexInput = { metaId?: MetaId; /** Tags for filtering entries in Storybook and its tools. */ tags?: Tag[]; + /** Tags from the meta for filtering entries in Storybook and its tools. */ + metaTags?: Tag[]; /** * The id of the entry, auto-generated from {@link title}/{@link metaId} and {@link exportName} if unspecified. * If specified, the story in the CSF file _must_ have a matching id set at `parameters.__id`, to be correctly matched. From 99e3993e0e0f57e4d19c979e4b60a9f5e62e18ed Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 10 Jan 2024 20:51:53 +0800 Subject: [PATCH 2/7] Update snapshots --- .../src/utils/__tests__/index-extraction.test.ts | 5 ----- code/lib/csf-tools/src/CsfFile.test.ts | 10 ++++++++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/code/lib/core-server/src/utils/__tests__/index-extraction.test.ts b/code/lib/core-server/src/utils/__tests__/index-extraction.test.ts index 451a350755b2..8a3c43dfed73 100644 --- a/code/lib/core-server/src/utils/__tests__/index-extraction.test.ts +++ b/code/lib/core-server/src/utils/__tests__/index-extraction.test.ts @@ -406,7 +406,6 @@ describe('docs entries from story extraction', () => { "name": "docs", "storiesImports": [], "tags": [ - "story-tag-from-indexer", "docs", "autodocs", ], @@ -467,8 +466,6 @@ describe('docs entries from story extraction', () => { "name": "docs", "storiesImports": [], "tags": [ - "autodocs", - "story-tag-from-indexer", "docs", ], "title": "A", @@ -578,8 +575,6 @@ describe('docs entries from story extraction', () => { "name": "docs", "storiesImports": [], "tags": [ - "stories-mdx", - "story-tag-from-indexer", "docs", ], "title": "A", diff --git a/code/lib/csf-tools/src/CsfFile.test.ts b/code/lib/csf-tools/src/CsfFile.test.ts index c503cf795672..400e4306ccb6 100644 --- a/code/lib/csf-tools/src/CsfFile.test.ts +++ b/code/lib/csf-tools/src/CsfFile.test.ts @@ -1098,6 +1098,8 @@ describe('CsfFile', () => { - component-tag - story-tag - play-fn + metaTags: &ref_0 + - component-tag __id: component-id--a - type: story importPath: foo/bar.stories.js @@ -1109,6 +1111,7 @@ describe('CsfFile', () => { - component-tag - story-tag - play-fn + metaTags: *ref_0 __id: component-id--b `); }); @@ -1138,6 +1141,8 @@ describe('CsfFile', () => { metaId: component-id tags: - component-tag + metaTags: + - component-tag __id: custom-story-id `); }); @@ -1169,6 +1174,11 @@ describe('CsfFile', () => { - inherit-tag-dup - story-tag - story-tag-dup + metaTags: + - component-tag + - component-tag-dup + - component-tag-dup + - inherit-tag-dup __id: custom-foo-title--a `); }); From 7f1e89775ab8e8c072cf92ddfdb1ccb1c78afbe3 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 10 Jan 2024 23:57:18 +0800 Subject: [PATCH 3/7] Add e2e tests for tags filtering --- code/e2e-tests/tags.spec.ts | 48 +++++++++++++++++++ .../template/stories/tags.stories.ts | 14 +++++- .../template/stories/test-only-tag.stories.ts | 10 ++++ 3 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 code/e2e-tests/tags.spec.ts create mode 100644 code/lib/preview-api/template/stories/test-only-tag.stories.ts diff --git a/code/e2e-tests/tags.spec.ts b/code/e2e-tests/tags.spec.ts new file mode 100644 index 000000000000..6687ecc926ef --- /dev/null +++ b/code/e2e-tests/tags.spec.ts @@ -0,0 +1,48 @@ +import { test, expect } from '@playwright/test'; +import { SbPage } from './util'; + +const storybookUrl = process.env.STORYBOOK_URL || 'http://localhost:8001'; + +test.describe('tags', () => { + test.beforeEach(async ({ page }) => { + await page.goto(storybookUrl); + await new SbPage(page).waitUntilLoaded(); + }); + + test('should correctly filter dev-only, docs-only, test-only stories', async ({ page }) => { + const sbPage = new SbPage(page); + + await sbPage.navigateToStory('lib/preview-api/tags', 'docs'); + + // Sidebar should include dev-only and exclude docs-only and test-only + const devOnlyEntry = await page.locator('#lib-preview-api-tags--dev-only').all(); + expect(devOnlyEntry.length).toBe(1); + + const docsOnlyEntry = await page.locator('#lib-preview-api-tags--docs-only').all(); + expect(docsOnlyEntry.length).toBe(0); + + const testOnlyEntry = await page.locator('#lib-preview-api-tags--test-only').all(); + expect(testOnlyEntry.length).toBe(0); + + // Autodocs should include docs-only and exclude dev-only and test-only + const root = sbPage.previewRoot(); + + const devOnlyAnchor = await root.locator('#anchor--lib-preview-api-tags--dev-only').all(); + expect(devOnlyAnchor.length).toBe(0); + + const docsOnlyAnchor = await root.locator('#anchor--lib-preview-api-tags--docs-only').all(); + expect(docsOnlyAnchor.length).toBe(1); + + const testOnlyAnchor = await root.locator('#anchor--lib-preview-api-tags--test-only').all(); + expect(testOnlyAnchor.length).toBe(0); + }); + + test('should correctly filter out test-only autodocs pages', async ({ page }) => { + const sbPage = new SbPage(page); + await sbPage.selectToolbar('#lib-preview-api'); + + // Sidebar should exclude test-only stories and their docs + const componentEntry = await page.locator('#lib-preview-api-test-only-tag').all(); + expect(componentEntry.length).toBe(0); + }); +}); diff --git a/code/lib/preview-api/template/stories/tags.stories.ts b/code/lib/preview-api/template/stories/tags.stories.ts index bae16f25e678..76afff7be017 100644 --- a/code/lib/preview-api/template/stories/tags.stories.ts +++ b/code/lib/preview-api/template/stories/tags.stories.ts @@ -5,7 +5,7 @@ import { expect } from '@storybook/jest'; export default { component: globalThis.Components.Pre, - tags: ['component-one', 'component-two'], + tags: ['component-one', 'component-two', 'autodocs'], decorators: [ (storyFn: PartialStoryFn, context: StoryContext) => { return storyFn({ @@ -24,3 +24,15 @@ export const Inheritance = { }); }, }; + +export const DocsOnly = { + tags: ['docs-only'], +}; + +export const TestOnly = { + tags: ['test-only'], +}; + +export const DevOnly = { + tags: ['dev-only'], +}; diff --git a/code/lib/preview-api/template/stories/test-only-tag.stories.ts b/code/lib/preview-api/template/stories/test-only-tag.stories.ts new file mode 100644 index 000000000000..3e261ca6d760 --- /dev/null +++ b/code/lib/preview-api/template/stories/test-only-tag.stories.ts @@ -0,0 +1,10 @@ +import { global as globalThis } from '@storybook/global'; + +export default { + component: globalThis.Components.Button, + tags: ['autodocs', 'test-only'], +}; + +export const Default = { + args: { label: 'Button' }, +}; From f4e9cd28f203de898d82a1df4d16a7ed6da8fa48 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 11 Jan 2024 00:20:50 +0800 Subject: [PATCH 4/7] Disable some e2e stories from chromatic --- code/lib/preview-api/template/stories/tags.stories.ts | 2 ++ code/lib/preview-api/template/stories/test-only-tag.stories.ts | 1 + 2 files changed, 3 insertions(+) diff --git a/code/lib/preview-api/template/stories/tags.stories.ts b/code/lib/preview-api/template/stories/tags.stories.ts index 76afff7be017..d896124dffa6 100644 --- a/code/lib/preview-api/template/stories/tags.stories.ts +++ b/code/lib/preview-api/template/stories/tags.stories.ts @@ -13,6 +13,7 @@ export default { }); }, ], + parameters: { chromatic: { disable: true } }, }; export const Inheritance = { @@ -23,6 +24,7 @@ export const Inheritance = { tags: ['story-one', 'story-two', 'story'], }); }, + parameters: { chromatic: { disable: false } }, }; export const DocsOnly = { diff --git a/code/lib/preview-api/template/stories/test-only-tag.stories.ts b/code/lib/preview-api/template/stories/test-only-tag.stories.ts index 3e261ca6d760..138f221d3ff7 100644 --- a/code/lib/preview-api/template/stories/test-only-tag.stories.ts +++ b/code/lib/preview-api/template/stories/test-only-tag.stories.ts @@ -3,6 +3,7 @@ import { global as globalThis } from '@storybook/global'; export default { component: globalThis.Components.Button, tags: ['autodocs', 'test-only'], + parameters: { chromatic: { disable: true } }, }; export const Default = { From 3291cf288d68daf9ec206ac27280a25836aae2fc Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 11 Jan 2024 17:47:17 +0800 Subject: [PATCH 5/7] UI: Wait for globals before rendering UI for Webkit --- .../core-server/src/presets/common-manager.ts | 18 ++++++++++-------- code/ui/manager/src/runtime.ts | 7 ++++++- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/code/lib/core-server/src/presets/common-manager.ts b/code/lib/core-server/src/presets/common-manager.ts index f8ecf896d84d..081722917468 100644 --- a/code/lib/core-server/src/presets/common-manager.ts +++ b/code/lib/core-server/src/presets/common-manager.ts @@ -3,15 +3,17 @@ import { global } from '@storybook/global'; const STATIC_FILTER = 'static-filter'; -const excludeTags = Object.entries(global.TAGS_OPTIONS ?? {}).reduce((acc, entry) => { - const [tag, option] = entry; - if ((option as any).excludeFromSidebar) { - acc[tag] = true; - } - return acc; -}, {} as Record); - addons.register(STATIC_FILTER, (api) => { + // FIXME: this ensures the filter is applied after the first render + // to avoid a strange race condition in Webkit only. + const excludeTags = Object.entries(global.TAGS_OPTIONS ?? {}).reduce((acc, entry) => { + const [tag, option] = entry; + if ((option as any).excludeFromSidebar) { + acc[tag] = true; + } + return acc; + }, {} as Record); + api.experimental_setFilter(STATIC_FILTER, (item) => { const tags = item.tags || []; return tags.filter((tag) => excludeTags[tag]).length === 0; diff --git a/code/ui/manager/src/runtime.ts b/code/ui/manager/src/runtime.ts index 861fc3b88fcb..952f30693aa3 100644 --- a/code/ui/manager/src/runtime.ts +++ b/code/ui/manager/src/runtime.ts @@ -55,4 +55,9 @@ class ReactProvider extends Provider { const { document } = global; const rootEl = document.getElementById('root'); -renderStorybookUI(rootEl, new ReactProvider()); + +// We need to wait for the script tag containing the global objects +// to be run by Webkit before rendering the UI. This is fine in most browsers. +setTimeout(() => { + renderStorybookUI(rootEl, new ReactProvider()); +}, 0); From 7c1fae91e3187c2ebad30974042b63a36a0e75ec Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 11 Jan 2024 21:28:54 +0800 Subject: [PATCH 6/7] Add tests for navigating to stories/docs filtered from the sidebar --- code/e2e-tests/tags.spec.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/code/e2e-tests/tags.spec.ts b/code/e2e-tests/tags.spec.ts index 6687ecc926ef..81a2233e0700 100644 --- a/code/e2e-tests/tags.spec.ts +++ b/code/e2e-tests/tags.spec.ts @@ -44,5 +44,20 @@ test.describe('tags', () => { // Sidebar should exclude test-only stories and their docs const componentEntry = await page.locator('#lib-preview-api-test-only-tag').all(); expect(componentEntry.length).toBe(0); + + // Even though test-only autodocs not sidebar, it is still in the preview + // Even though the test-only story is filtered out of the stories, it is still the primary story (should it be?) + await sbPage.deepLinkToStory(storybookUrl, 'lib/preview-api/test-only-tag', 'docs'); + await sbPage.waitUntilLoaded(); + let root = sbPage.previewRoot(); + let button = await root.locator('button', { hasText: 'button' }); + expect(button).toBeVisible(); + + // Even though test-only story not sidebar, it is still in the preview + await sbPage.deepLinkToStory(storybookUrl, 'lib/preview-api/test-only-tag', 'default'); + await sbPage.waitUntilLoaded(); + root = sbPage.previewRoot(); + button = await root.locator('button', { hasText: 'button' }); + expect(button).toBeVisible(); }); }); From 185e342944949ee562501c211f4f6d45f7921c32 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Fri, 12 Jan 2024 19:52:27 +0800 Subject: [PATCH 7/7] Fix e2e tests for navigating to filtered pages --- code/e2e-tests/tags.spec.ts | 11 +++++------ code/e2e-tests/util.ts | 3 +++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/code/e2e-tests/tags.spec.ts b/code/e2e-tests/tags.spec.ts index 81a2233e0700..37fb76fb814c 100644 --- a/code/e2e-tests/tags.spec.ts +++ b/code/e2e-tests/tags.spec.ts @@ -39,6 +39,7 @@ test.describe('tags', () => { test('should correctly filter out test-only autodocs pages', async ({ page }) => { const sbPage = new SbPage(page); + await sbPage.selectToolbar('#lib-preview-api'); // Sidebar should exclude test-only stories and their docs @@ -49,15 +50,13 @@ test.describe('tags', () => { // Even though the test-only story is filtered out of the stories, it is still the primary story (should it be?) await sbPage.deepLinkToStory(storybookUrl, 'lib/preview-api/test-only-tag', 'docs'); await sbPage.waitUntilLoaded(); - let root = sbPage.previewRoot(); - let button = await root.locator('button', { hasText: 'button' }); - expect(button).toBeVisible(); + const docsButton = await sbPage.previewRoot().locator('button', { hasText: 'Button' }); + await expect(docsButton).toBeVisible(); // Even though test-only story not sidebar, it is still in the preview await sbPage.deepLinkToStory(storybookUrl, 'lib/preview-api/test-only-tag', 'default'); await sbPage.waitUntilLoaded(); - root = sbPage.previewRoot(); - button = await root.locator('button', { hasText: 'button' }); - expect(button).toBeVisible(); + const storyButton = await sbPage.previewRoot().locator('button', { hasText: 'Button' }); + await expect(storyButton).toBeVisible(); }); }); diff --git a/code/e2e-tests/util.ts b/code/e2e-tests/util.ts index 38b0e78cb4c8..ce4cd6b09af0 100644 --- a/code/e2e-tests/util.ts +++ b/code/e2e-tests/util.ts @@ -33,6 +33,9 @@ export class SbPage { const storyLinkId = `${titleId}--${storyId}`; const viewMode = name === 'docs' ? 'docs' : 'story'; await this.page.goto(`${baseURL}/?path=/${viewMode}/${storyLinkId}`); + + await this.page.waitForURL((url) => url.search.includes(`path=/${viewMode}/${storyLinkId}`)); + await this.previewRoot(); } /**