From 42a35ae3e80c9dd3f85a490268917299f78e6de3 Mon Sep 17 00:00:00 2001 From: Aron Griffis Date: Thu, 30 Mar 2023 14:35:06 -0400 Subject: [PATCH 1/6] fix: handle button.group.stories.js in auto-title The stripExtension() function aggressively removed everything after the first dot in the filename. It should remove only (for example) .js or .stories.js or .story.js See the new test in autoTitle.test.ts for example. Additionally the stripExtension() function was pulling double-duty by cleaning up an empty leading string, a leftover from calling pathJoin([titlePrefix, suffix]) with empty titlePrefix. Handle this properly in the earlier code instead of hacking stripExtension(). --- .../src/modules/store/autoTitle.test.ts | 10 ++++ .../src/modules/store/autoTitle.ts | 46 +++++++------------ 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/code/lib/preview-api/src/modules/store/autoTitle.test.ts b/code/lib/preview-api/src/modules/store/autoTitle.test.ts index e4c1fe23dbb2..c117b44a05a9 100644 --- a/code/lib/preview-api/src/modules/store/autoTitle.test.ts +++ b/code/lib/preview-api/src/modules/store/autoTitle.test.ts @@ -187,6 +187,16 @@ describe('userOrAutoTitleFromSpecifier', () => { ).toMatchInlineSnapshot(`to/button`); }); + it('match with dotted component', () => { + expect( + userOrAuto( + './path/to/button/button.group.stories.js', + normalizeStoriesEntry({ directory: './path' }, options), + undefined + ) + ).toMatchInlineSnapshot(`to/button/button.group`); + }); + it('match with hyphen path', () => { expect( userOrAuto( diff --git a/code/lib/preview-api/src/modules/store/autoTitle.ts b/code/lib/preview-api/src/modules/store/autoTitle.ts index ca810f63bbb9..c28125f65ff4 100644 --- a/code/lib/preview-api/src/modules/store/autoTitle.ts +++ b/code/lib/preview-api/src/modules/store/autoTitle.ts @@ -6,31 +6,16 @@ import type { NormalizedStoriesSpecifier } from '@storybook/types'; // FIXME: types duplicated type from `core-common', to be // removed when we remove v6 back-compat. -const stripExtension = (path: string[]) => { - let parts = [...path]; - const last = parts[parts.length - 1]; - const dotIndex = last.indexOf('.'); - const stripped = dotIndex > 0 ? last.substr(0, dotIndex) : last; - parts[parts.length - 1] = stripped; - const [first, ...rest] = parts; - if (first === '') { - parts = rest; - } - return parts; +const stripExtension = (parts: string[]) => { + const last = parts[parts.length - 1]?.replace(/(?:[.](?:story|stories))?([.][^.]+)$/i, ''); + return last ? [...parts.slice(0, -1), last] : parts; }; -const indexRe = /^index$/i; - // deal with files like "atoms/button/{button,index}.stories.js" -const removeRedundantFilename = (paths: string[]) => { - let prevVal: string; - return paths.filter((val, index) => { - if (index === paths.length - 1 && (val === prevVal || indexRe.test(val))) { - return false; - } - prevVal = val; - return true; - }); +const removeRedundantFilename = (parts: string[]) => { + const last = parts[parts.length - 1]; + const nextToLast = parts[parts.length - 2]; + return last && (last === nextToLast || /^index$/i.test(last)) ? parts.slice(0, -1) : parts; }; /** @@ -41,8 +26,10 @@ const removeRedundantFilename = (paths: string[]) => { * @returns joined path string, with single '/' between parts */ function pathJoin(paths: string[]): string { - const slashes = new RegExp('/{1,}', 'g'); - return paths.join('/').replace(slashes, '/'); + return paths + .flatMap((p) => p.split('/')) + .filter(Boolean) + .join('/'); } export const userOrAutoTitleFromSpecifier = ( @@ -67,18 +54,17 @@ export const userOrAutoTitleFromSpecifier = ( if (importPathMatcher.exec(normalizedFileName)) { if (!userTitle) { const suffix = normalizedFileName.replace(directory, ''); - const titleAndSuffix = slash(pathJoin([titlePrefix, suffix])); - let path = titleAndSuffix.split('/'); - path = stripExtension(path); - path = removeRedundantFilename(path); - return path.join('/'); + let parts = pathJoin([titlePrefix, suffix]).split('/'); + parts = stripExtension(parts); + parts = removeRedundantFilename(parts); + return parts.join('/'); } if (!titlePrefix) { return userTitle; } - return slash(pathJoin([titlePrefix, userTitle])); + return pathJoin([titlePrefix, userTitle]); } return undefined; From 8ce45d5d680926e769692c30ec238ec160288a17 Mon Sep 17 00:00:00 2001 From: Aron Griffis Date: Thu, 30 Mar 2023 15:08:19 -0400 Subject: [PATCH 2/6] feat: handle stories.js as meaningless for auto-titling --- .../src/modules/store/autoTitle.test.ts | 23 +++++++++++++++++++ .../src/modules/store/autoTitle.ts | 4 +++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/code/lib/preview-api/src/modules/store/autoTitle.test.ts b/code/lib/preview-api/src/modules/store/autoTitle.test.ts index c117b44a05a9..013ccac6690f 100644 --- a/code/lib/preview-api/src/modules/store/autoTitle.test.ts +++ b/code/lib/preview-api/src/modules/store/autoTitle.test.ts @@ -187,6 +187,29 @@ describe('userOrAutoTitleFromSpecifier', () => { ).toMatchInlineSnapshot(`to/button`); }); + it('match with trailing stories', () => { + expect( + userOrAuto( + './path/to/button/stories.js', + normalizeStoriesEntry({ directory: './path', files: '**/?(*.)stories.*' }, options), + undefined + ) + ).toMatchInlineSnapshot(`to/button`); + }); + + it('match with trailing stories (windows path)', () => { + expect( + userOrAuto( + './path/to/button/stories.js', + normalizeStoriesEntry( + { directory: '.\\path\\', files: '**/?(*.)stories.*' }, + winOptions + ), + undefined + ) + ).toMatchInlineSnapshot(`to/button`); + }); + it('match with dotted component', () => { expect( userOrAuto( diff --git a/code/lib/preview-api/src/modules/store/autoTitle.ts b/code/lib/preview-api/src/modules/store/autoTitle.ts index c28125f65ff4..85fff681ddf2 100644 --- a/code/lib/preview-api/src/modules/store/autoTitle.ts +++ b/code/lib/preview-api/src/modules/store/autoTitle.ts @@ -15,7 +15,9 @@ const stripExtension = (parts: string[]) => { const removeRedundantFilename = (parts: string[]) => { const last = parts[parts.length - 1]; const nextToLast = parts[parts.length - 2]; - return last && (last === nextToLast || /^index$/i.test(last)) ? parts.slice(0, -1) : parts; + return last && (last === nextToLast || /^(?:index|story|stories)$/i.test(last)) + ? parts.slice(0, -1) + : parts; }; /** From 59fc32929ba3f5a8573b1d9c11a2edb80618b81d Mon Sep 17 00:00:00 2001 From: Aron Griffis Date: Wed, 20 Sep 2023 16:15:12 -0400 Subject: [PATCH 3/6] fix: don't remove redundant filename part if there won't be anything left --- .../preview-api/src/modules/store/autoTitle.test.ts | 12 ++++++++++++ code/lib/preview-api/src/modules/store/autoTitle.ts | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/code/lib/preview-api/src/modules/store/autoTitle.test.ts b/code/lib/preview-api/src/modules/store/autoTitle.test.ts index 013ccac6690f..312f1bced806 100644 --- a/code/lib/preview-api/src/modules/store/autoTitle.test.ts +++ b/code/lib/preview-api/src/modules/store/autoTitle.test.ts @@ -240,6 +240,18 @@ describe('userOrAutoTitleFromSpecifier', () => { ).toMatchInlineSnapshot(`to_my/file`); }); + it('match with short path', () => { + // Make sure "stories" isn't trimmed as redundant when there won't be + // anything left. + expect( + userOrAuto( + './path/stories.js', + normalizeStoriesEntry({ directory: './path', files: '**/?(*.)stories.*' }, options), + undefined + ) + ).toMatchInlineSnapshot(`stories`); + }); + it('match with windows path', () => { expect( userOrAuto( diff --git a/code/lib/preview-api/src/modules/store/autoTitle.ts b/code/lib/preview-api/src/modules/store/autoTitle.ts index 85fff681ddf2..72965b06aad4 100644 --- a/code/lib/preview-api/src/modules/store/autoTitle.ts +++ b/code/lib/preview-api/src/modules/store/autoTitle.ts @@ -15,7 +15,7 @@ const stripExtension = (parts: string[]) => { const removeRedundantFilename = (parts: string[]) => { const last = parts[parts.length - 1]; const nextToLast = parts[parts.length - 2]; - return last && (last === nextToLast || /^(?:index|story|stories)$/i.test(last)) + return last && nextToLast && (last === nextToLast || /^(?:index|story|stories)$/i.test(last)) ? parts.slice(0, -1) : parts; }; From fcecff36cca30de1fe0415d037b68664b54aef92 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 1 Nov 2023 17:16:16 +0800 Subject: [PATCH 4/6] Add 8.0 autotitle migration docs --- MIGRATION.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/MIGRATION.md b/MIGRATION.md index ccfbcfe0105c..ce3574e0e216 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -2,6 +2,7 @@ - [From version 7.x to 8.0.0](#from-version-7x-to-800) - [Core changes](#core-changes) + - [Autotitle breaking fixes](#autotitle-breaking-fixes) - [UI layout state has changed shape](#ui-layout-state-has-changed-shape) - [New UI and props for Button and IconButton components](#new-ui-and-props-for-button-and-iconbutton-components) - [Icons is deprecated](#icons-is-deprecated) @@ -316,6 +317,16 @@ ### Core changes +#### Autotitle breaking fixes + +In Storybook 7, the file name `path/to/foo.bar.stories.js` would result in the [autotitle](https://storybook.js.org/docs/react/configure/overview#configure-story-loading) `path/to/foo`. In 8.0, this has been changed to generate `path/to/foo.bar`. We consider this a bugfix but it is also a breaking change if you depended on the old behavior. To get the old titles, you can manually specify the desired title in the default export of your component file. For example: + +```js +export default { + title: 'path/to/foo', +} +``` + #### UI layout state has changed shape In Storybook 7 it was possible to use `addons.setConfig({...});` to configure Storybook UI features and behavior as documented [here (v7)](https://storybook.js.org/docs/7.3/react/configure/features-and-behavior), [(latest)](https://storybook.js.org/docs/react/configure/features-and-behavior). The state and API for the UI layout has changed: From cafe331ea6c5cad0bd6ef3ca0f6b7a1c14f0ca14 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 1 Nov 2023 20:22:05 +0800 Subject: [PATCH 5/6] Improved migration --- MIGRATION.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/MIGRATION.md b/MIGRATION.md index ce3574e0e216..f80790a9cdf5 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -319,7 +319,7 @@ #### Autotitle breaking fixes -In Storybook 7, the file name `path/to/foo.bar.stories.js` would result in the [autotitle](https://storybook.js.org/docs/react/configure/overview#configure-story-loading) `path/to/foo`. In 8.0, this has been changed to generate `path/to/foo.bar`. We consider this a bugfix but it is also a breaking change if you depended on the old behavior. To get the old titles, you can manually specify the desired title in the default export of your component file. For example: +In Storybook 7, the file name `path/to/foo.bar.stories.js` would result in the [autotitle](https://storybook.js.org/docs/react/configure/overview#configure-story-loading) `path/to/foo`. In 8.0, this has been changed to generate `path/to/foo.bar`. We consider this a bugfix but it is also a breaking change if you depended on the old behavior. To get the old titles, you can manually specify the desired title in the default export of your story file. For example: ```js export default { @@ -327,6 +327,8 @@ export default { } ``` +Alternatively, if you need to achieve a different behavior for a large number of files, you can provide a [custom indexer](https://storybook.js.org/docs/7.0/vue/configure/sidebar-and-urls#processing-custom-titles) to generate the titles dynamically. + #### UI layout state has changed shape In Storybook 7 it was possible to use `addons.setConfig({...});` to configure Storybook UI features and behavior as documented [here (v7)](https://storybook.js.org/docs/7.3/react/configure/features-and-behavior), [(latest)](https://storybook.js.org/docs/react/configure/features-and-behavior). The state and API for the UI layout has changed: From e15dd64cae8bffbd2490bd39e20988b29bb8e1a4 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 2 Nov 2023 17:46:19 +0800 Subject: [PATCH 6/6] Fix corner case Story.stories.js --- .../src/modules/store/autoTitle.test.ts | 16 +++++++++++++ .../src/modules/store/autoTitle.ts | 23 +++++++++++-------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/code/lib/preview-api/src/modules/store/autoTitle.test.ts b/code/lib/preview-api/src/modules/store/autoTitle.test.ts index 312f1bced806..9f6089342225 100644 --- a/code/lib/preview-api/src/modules/store/autoTitle.test.ts +++ b/code/lib/preview-api/src/modules/store/autoTitle.test.ts @@ -324,5 +324,21 @@ describe('userOrAutoTitleFromSpecifier', () => { ).toMatchInlineSnapshot(`to_my/MyButton`); }); }); + + it('Story.stories.js', () => { + expect( + userOrAuto( + '../blocks/src/Story.stories.tsx', + normalizeStoriesEntry( + { + directory: '../blocks/src', + titlePrefix: '@blocks', + }, + options + ), + undefined + ) + ).toMatchInlineSnapshot(`@blocks/Story`); + }); }); }); diff --git a/code/lib/preview-api/src/modules/store/autoTitle.ts b/code/lib/preview-api/src/modules/store/autoTitle.ts index 72965b06aad4..b925c7fc3d24 100644 --- a/code/lib/preview-api/src/modules/store/autoTitle.ts +++ b/code/lib/preview-api/src/modules/store/autoTitle.ts @@ -6,18 +6,22 @@ import type { NormalizedStoriesSpecifier } from '@storybook/types'; // FIXME: types duplicated type from `core-common', to be // removed when we remove v6 back-compat. -const stripExtension = (parts: string[]) => { - const last = parts[parts.length - 1]?.replace(/(?:[.](?:story|stories))?([.][^.]+)$/i, ''); - return last ? [...parts.slice(0, -1), last] : parts; -}; - // deal with files like "atoms/button/{button,index}.stories.js" -const removeRedundantFilename = (parts: string[]) => { +const sanitize = (parts: string[]) => { + if (parts.length === 0) return parts; + const last = parts[parts.length - 1]; + const lastStripped = last?.replace(/(?:[.](?:story|stories))?([.][^.]+)$/i, ''); + if (parts.length === 1) return [lastStripped]; + const nextToLast = parts[parts.length - 2]; - return last && nextToLast && (last === nextToLast || /^(?:index|story|stories)$/i.test(last)) + return lastStripped && + nextToLast && + (lastStripped === nextToLast || + /^(story|stories)([.][^.]+)$/i.test(last) || + /^index$/i.test(lastStripped)) ? parts.slice(0, -1) - : parts; + : [...parts.slice(0, -1), lastStripped]; }; /** @@ -57,8 +61,7 @@ export const userOrAutoTitleFromSpecifier = ( if (!userTitle) { const suffix = normalizedFileName.replace(directory, ''); let parts = pathJoin([titlePrefix, suffix]).split('/'); - parts = stripExtension(parts); - parts = removeRedundantFilename(parts); + parts = sanitize(parts); return parts.join('/'); }