From e90f70ad3bfe503a7fd7260641aa65d0578ec0c2 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 24 Apr 2019 16:35:38 +0800 Subject: [PATCH 1/8] Refactor splitPath into router utils --- lib/api/src/modules/stories.ts | 18 +----------------- lib/client-api/src/index.js | 3 +-- lib/client-api/src/story_store.js | 11 ----------- lib/router/src/utils.ts | 20 ++++++++++++++++++-- 4 files changed, 20 insertions(+), 32 deletions(-) diff --git a/lib/api/src/modules/stories.ts b/lib/api/src/modules/stories.ts index de9ecf450690..c60f4bba232c 100644 --- a/lib/api/src/modules/stories.ts +++ b/lib/api/src/modules/stories.ts @@ -1,4 +1,4 @@ -import { toId, sanitize } from '@storybook/router/dist/utils'; +import { toId, sanitize, splitPath } from '@storybook/router/utils'; import { Module } from '../index'; import merge from '../lib/merge'; @@ -27,11 +27,6 @@ export interface SubAPI { getParameters: (storyId: StoryId, parameterName?: ParameterName) => Story['parameters'] | any; } -interface SeparatorOptions { - rootSeparator: string | RegExp; - groupSeparator: string | RegExp; -} - interface Group { id: StoryId; name: string; @@ -164,17 +159,6 @@ const initStoriesApi = ({ navigate(`/${viewMode || 'story'}/${result}`); }; - const splitPath = (kind: string, { rootSeparator, groupSeparator }: SeparatorOptions) => { - const [root, remainder] = kind.split(rootSeparator, 2); - const groups = (remainder || kind).split(groupSeparator).filter(i => !!i); - - // when there's no remainder, it means the root wasn't found/split - return { - root: remainder ? root : null, - groups, - }; - }; - const toKey = (input: string) => input.replace(/[^a-z0-9]+([a-z0-9])/gi, (...params) => params[1].toUpperCase()); diff --git a/lib/client-api/src/index.js b/lib/client-api/src/index.js index 357f98112b73..23934980e126 100644 --- a/lib/client-api/src/index.js +++ b/lib/client-api/src/index.js @@ -1,5 +1,5 @@ import ClientApi, { defaultDecorateStory } from './client_api'; -import StoryStore, { splitPath } from './story_store'; +import StoryStore from './story_store'; import ConfigApi from './config_api'; import subscriptionsStore from './subscriptions_store'; import pathToId from './pathToId'; @@ -13,7 +13,6 @@ export { subscriptionsStore, defaultDecorateStory, pathToId, - splitPath, getQueryParams, getQueryParam, }; diff --git a/lib/client-api/src/story_store.js b/lib/client-api/src/story_store.js index cdc6a6d9c337..a72e696a1782 100644 --- a/lib/client-api/src/story_store.js +++ b/lib/client-api/src/story_store.js @@ -21,17 +21,6 @@ const toKey = input => const toChild = it => ({ ...it }); -export const splitPath = (path, { rootSeparator, groupSeparator }) => { - const [root, remainder] = path.split(rootSeparator, 2); - const groups = (remainder || path).split(groupSeparator).filter(i => !!i); - - // when there's no remainder, it means the root wasn't found/split - return { - root: remainder ? root : null, - groups, - }; -}; - let count = 0; function getId() { diff --git a/lib/router/src/utils.ts b/lib/router/src/utils.ts index 6b2056a9d432..6e88016242f1 100644 --- a/lib/router/src/utils.ts +++ b/lib/router/src/utils.ts @@ -6,8 +6,13 @@ interface StoryData { storyId?: string; } +interface SeparatorOptions { + rootSeparator: string | RegExp; + groupSeparator: string | RegExp; +} + export const knownNonViewModesRegex = /(settings)/; -const splitPath = /\/([^/]+)\/([^/]+)?/; +const splitPathRegex = /\/([^/]+)\/([^/]+)?/; // Remove punctuation https://gist.github.com/davidjrice/9d2af51100e41c6c4b4a export const sanitize = (string: string) => { @@ -38,7 +43,7 @@ export const storyDataFromString: (path?: string) => StoryData = memoize(1000)( }; if (path) { - const [, viewMode, storyId] = path.match(splitPath) || [undefined, undefined, undefined]; + const [, viewMode, storyId] = path.match(splitPathRegex) || [undefined, undefined, undefined]; if (viewMode && !viewMode.match(knownNonViewModesRegex)) { Object.assign(result, { viewMode, @@ -73,3 +78,14 @@ export const getMatch = memoize(1000)( return null; } ); + +export const splitPath = (kind: string, { rootSeparator, groupSeparator }: SeparatorOptions) => { + const [root, remainder] = kind.split(rootSeparator, 2); + const groups = (remainder || kind).split(groupSeparator).filter(i => !!sanitize(i)); + + // when there's no remainder, it means the root wasn't found/split + return { + root: remainder ? root : null, + groups, + }; +}; From 9141b8ef43fbf76f96e216d6528b60604098a627 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 24 Apr 2019 16:37:11 +0800 Subject: [PATCH 2/8] Throw an error instead of running into an infinite loop --- lib/api/src/modules/stories.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/api/src/modules/stories.ts b/lib/api/src/modules/stories.ts index c60f4bba232c..b72e3268d6d8 100644 --- a/lib/api/src/modules/stories.ts +++ b/lib/api/src/modules/stories.ts @@ -192,6 +192,15 @@ const initStoriesApi = ({ const { name } = group; const parent = index > 0 && soFar[index - 1].id; const id = sanitize(parent ? `${parent}-${name}` : name); + if (parent === id) { + throw new Error( + ` +Invalid kind with id === parentId: '${id}' + +Did you create a path that uses the separator char accidentally, such as 'Vue ' where '/' is a separator char? See https://github.com/storybooks/storybook/issues/6128 + `.trim() + ); + } const result: Group = { ...group, From 66052bc42d548e938c5b20ccd04ec361172500df Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 24 Apr 2019 16:37:48 +0800 Subject: [PATCH 3/8] Fix an unrelated typo that probably causes a bug in RN server --- lib/api/src/modules/stories.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/src/modules/stories.ts b/lib/api/src/modules/stories.ts index b72e3268d6d8..f853768448f7 100644 --- a/lib/api/src/modules/stories.ts +++ b/lib/api/src/modules/stories.ts @@ -176,7 +176,7 @@ const initStoriesApi = ({ hierarchyRootSeparator: rootSeparator, hierarchySeparator: groupSeparator, } = (parameters && parameters.options) || { - hierarchyRootSeparator: '/', + hierarchyRootSeparator: '|', hierarchySeparator: '/', }; From 2750e85ca4a1fafb50fc30236ba767ab64662743 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 24 Apr 2019 16:40:50 +0800 Subject: [PATCH 4/8] Remove sanitization - this masks an error in the API layer --- lib/router/src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/router/src/utils.ts b/lib/router/src/utils.ts index 6e88016242f1..e0eb8d0ce2bf 100644 --- a/lib/router/src/utils.ts +++ b/lib/router/src/utils.ts @@ -81,7 +81,7 @@ export const getMatch = memoize(1000)( export const splitPath = (kind: string, { rootSeparator, groupSeparator }: SeparatorOptions) => { const [root, remainder] = kind.split(rootSeparator, 2); - const groups = (remainder || kind).split(groupSeparator).filter(i => !!sanitize(i)); + const groups = (remainder || kind).split(groupSeparator).filter(i => !!i); // when there's no remainder, it means the root wasn't found/split return { From 212362acb50f19359f3d7096edcaaed4c3bd76c3 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 24 Apr 2019 18:00:20 +0800 Subject: [PATCH 5/8] Fix typescript type error --- lib/api/src/modules/stories.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/api/src/modules/stories.ts b/lib/api/src/modules/stories.ts index f853768448f7..c71c3f435102 100644 --- a/lib/api/src/modules/stories.ts +++ b/lib/api/src/modules/stories.ts @@ -1,4 +1,5 @@ -import { toId, sanitize, splitPath } from '@storybook/router/utils'; +// FIXME: we shouldn't import from dist but there are no types otherwise +import { toId, sanitize, splitPath } from '@storybook/router/dist/utils'; import { Module } from '../index'; import merge from '../lib/merge'; From 1c49d6558ce9c644f377eab215bb3fc7d26d52f7 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 24 Apr 2019 18:20:50 +0800 Subject: [PATCH 6/8] Rename some utilities to better reflect their functions --- lib/api/src/modules/stories.ts | 4 ++-- lib/router/src/router.tsx | 4 ++-- lib/router/src/utils.ts | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/api/src/modules/stories.ts b/lib/api/src/modules/stories.ts index c71c3f435102..a7e4a86d591f 100644 --- a/lib/api/src/modules/stories.ts +++ b/lib/api/src/modules/stories.ts @@ -1,5 +1,5 @@ // FIXME: we shouldn't import from dist but there are no types otherwise -import { toId, sanitize, splitPath } from '@storybook/router/dist/utils'; +import { toId, sanitize, parseKind } from '@storybook/router/dist/utils'; import { Module } from '../index'; import merge from '../lib/merge'; @@ -181,7 +181,7 @@ const initStoriesApi = ({ hierarchySeparator: '/', }; - const { root, groups } = splitPath(kind, { rootSeparator, groupSeparator }); + const { root, groups } = parseKind(kind, { rootSeparator, groupSeparator }); const rootAndGroups = [] .concat(root || []) diff --git a/lib/router/src/router.tsx b/lib/router/src/router.tsx index 7a1bdcdb0187..be408369c682 100644 --- a/lib/router/src/router.tsx +++ b/lib/router/src/router.tsx @@ -3,7 +3,7 @@ import React from 'react'; import { Link, Location, navigate, LocationProvider, RouteComponentProps } from '@reach/router'; import { ToggleVisibility } from './visibility'; -import { queryFromString, storyDataFromString, getMatch } from './utils'; +import { queryFromString, parsePath, getMatch } from './utils'; interface Other { viewMode?: string; @@ -56,7 +56,7 @@ const QueryLocation = ({ children }: QueryLocationProps) => ( {({ location }: RouteComponentProps): React.ReactNode => { const { path } = queryFromString(location.search); - const { viewMode, storyId } = storyDataFromString(path); + const { viewMode, storyId } = parsePath(path); return children({ path, location, navigate: queryNavigate, viewMode, storyId }); }} diff --git a/lib/router/src/utils.ts b/lib/router/src/utils.ts index e0eb8d0ce2bf..5a5a3f9fe074 100644 --- a/lib/router/src/utils.ts +++ b/lib/router/src/utils.ts @@ -35,7 +35,7 @@ const sanitizeSafe = (string: string, part: string) => { export const toId = (kind: string, name: string) => `${sanitizeSafe(kind, 'kind')}--${sanitizeSafe(name, 'name')}`; -export const storyDataFromString: (path?: string) => StoryData = memoize(1000)( +export const parsePath: (path?: string) => StoryData = memoize(1000)( (path: string | undefined | null) => { const result: StoryData = { viewMode: undefined, @@ -79,7 +79,7 @@ export const getMatch = memoize(1000)( } ); -export const splitPath = (kind: string, { rootSeparator, groupSeparator }: SeparatorOptions) => { +export const parseKind = (kind: string, { rootSeparator, groupSeparator }: SeparatorOptions) => { const [root, remainder] = kind.split(rootSeparator, 2); const groups = (remainder || kind).split(groupSeparator).filter(i => !!i); From e6fb6fd82218bfecb9e7a07df655f974f39d2609 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Wed, 24 Apr 2019 18:24:05 +0800 Subject: [PATCH 7/8] Improve error logging --- lib/api/src/modules/stories.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/src/modules/stories.ts b/lib/api/src/modules/stories.ts index a7e4a86d591f..d88e43be4faa 100644 --- a/lib/api/src/modules/stories.ts +++ b/lib/api/src/modules/stories.ts @@ -196,7 +196,7 @@ const initStoriesApi = ({ if (parent === id) { throw new Error( ` -Invalid kind with id === parentId: '${id}' +Invalid part '${name}', leading to id === parentId ('${id}'), inside kind '${kind}' Did you create a path that uses the separator char accidentally, such as 'Vue ' where '/' is a separator char? See https://github.com/storybooks/storybook/issues/6128 `.trim() From dabc1612b87532a361d3746ea154a44e68f3bda8 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Fri, 26 Apr 2019 16:27:44 +0800 Subject: [PATCH 8/8] Don't import from dist --- lib/api/src/modules/stories.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/src/modules/stories.ts b/lib/api/src/modules/stories.ts index d88e43be4faa..e8c98d175f3c 100644 --- a/lib/api/src/modules/stories.ts +++ b/lib/api/src/modules/stories.ts @@ -1,5 +1,5 @@ // FIXME: we shouldn't import from dist but there are no types otherwise -import { toId, sanitize, parseKind } from '@storybook/router/dist/utils'; +import { toId, sanitize, parseKind } from '@storybook/router'; import { Module } from '../index'; import merge from '../lib/merge';