Skip to content

Commit

Permalink
Merge pull request #13275 from storybookjs/13251-trim-component-name
Browse files Browse the repository at this point in the history
Core: Ensure node `name` does not contain leading/trailing whitespace
  • Loading branch information
shilman committed Dec 1, 2020
2 parents 9453155 + 627a127 commit e530ffb
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 95 deletions.
160 changes: 69 additions & 91 deletions lib/api/src/lib/stories.ts
Expand Up @@ -117,14 +117,6 @@ const warnChangedDefaultHierarchySeparators = deprecate(
`
);

const toKey = (input: string) =>
input.replace(/[^a-z0-9]+([a-z0-9])/gi, (...params) => params[1].toUpperCase());

const toGroup = (name: string) => ({
name,
id: toKey(name),
});

export const denormalizeStoryParameters = ({
globalParameters,
kindParameters,
Expand All @@ -144,103 +136,89 @@ export const transformStoriesRawToStoriesHash = (
input: StoriesRaw,
{ provider }: { provider: Provider }
): StoriesHash => {
const anyKindMatchesOldHierarchySeparators = Object.values(input)
.filter(Boolean)
.some(({ kind }) => kind.match(/\.|\|/));
const values = Object.values(input).filter(Boolean);
const usesOldHierarchySeparator = values.some(({ kind }) => kind.match(/\.|\|/)); // dot or pipe

const storiesHashOutOfOrder = Object.values(input)
.filter(Boolean)
.reduce((acc, item) => {
const { kind, parameters } = item;
const { showRoots } = provider.getConfig();
const storiesHashOutOfOrder = values.reduce((acc, item) => {
const { kind, parameters } = item;
const { showRoots } = provider.getConfig();

const setShowRoots = typeof showRoots !== 'undefined';
if (anyKindMatchesOldHierarchySeparators && !setShowRoots) {
warnChangedDefaultHierarchySeparators();
}
const setShowRoots = typeof showRoots !== 'undefined';
if (usesOldHierarchySeparator && !setShowRoots) {
warnChangedDefaultHierarchySeparators();
}

let root = '';
let groups: string[];
const parts: string[] = kind.split('/');
// Default showRoots to true if they didn't set it.
if ((!setShowRoots || showRoots) && parts.length > 1) {
[root, ...groups] = parts;
} else {
groups = parts;
}
const groups = kind.split('/').map((part) => part.trim());
const root = (!setShowRoots || showRoots) && groups.length > 1 ? [groups.shift()] : [];

const rootAndGroups = [...root, ...groups].reduce((list, name, index) => {
const parent = index > 0 && list[index - 1].id;
const id = sanitize(parent ? `${parent}-${name}` : name);

const rootAndGroups = []
.concat(root || [])
.concat(groups)
.map(toGroup)
// Map a bunch of extra fields onto the groups, collecting the path as we go (thus the reduce)
.reduce((soFar, group, index, original) => {
const { name } = group;
const parent = index > 0 && soFar[index - 1].id;
const id = sanitize(parent ? `${parent}-${name}` : name);
if (parent === id) {
throw new Error(
dedent`
if (parent === id) {
throw new Error(
dedent`
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 <docs/>' where '/' is a separator char? See https://github.com/storybookjs/storybook/issues/6128
`
);
}
);
}

if (!!root && index === 0) {
const result: Root = {
...group,
id,
depth: index,
children: [],
isComponent: false,
isLeaf: false,
isRoot: true,
};
return soFar.concat([result]);
}
const result: Group = {
...group,
id,
parent,
depth: index,
children: [],
isComponent: false,
isLeaf: false,
isRoot: false,
parameters: {
docsOnly: parameters?.docsOnly,
viewMode: parameters?.viewMode,
},
};
return soFar.concat([result]);
}, [] as GroupsList);
if (root.length && index === 0) {
list.push({
id,
name,
depth: index,
children: [],
isComponent: false,
isLeaf: false,
isRoot: true,
});
} else {
list.push({
id,
name,
parent,
depth: index,
children: [],
isComponent: false,
isLeaf: false,
isRoot: false,
parameters: {
docsOnly: parameters?.docsOnly,
viewMode: parameters?.viewMode,
},
});
}

const paths = [...rootAndGroups.map((g) => g.id), item.id];
return list;
}, [] as GroupsList);

// Ok, now let's add everything to the store
rootAndGroups.forEach((group, index) => {
const child = paths[index + 1];
const { id } = group;
acc[id] = merge(acc[id] || {}, {
...group,
...(child && { children: [child] }),
});
const paths = [...rootAndGroups.map(({ id }) => id), item.id];

// Ok, now let's add everything to the store
rootAndGroups.forEach((group, index) => {
const child = paths[index + 1];
const { id } = group;
acc[id] = merge(acc[id] || {}, {
...group,
...(child && { children: [child] }),
});
});

const story: Story = {
...item,
depth: rootAndGroups.length,
parent: rootAndGroups[rootAndGroups.length - 1].id,
isLeaf: true,
isComponent: false,
isRoot: false,
};
acc[item.id] = story;
const story: Story = {
...item,
depth: rootAndGroups.length,
parent: rootAndGroups[rootAndGroups.length - 1].id,
isLeaf: true,
isComponent: false,
isRoot: false,
};
acc[item.id] = story;

return acc;
}, {} as StoriesHash);
return acc;
}, {} as StoriesHash);

function addItem(acc: StoriesHash, item: Story | Group) {
if (!acc[item.id]) {
Expand Down
42 changes: 42 additions & 0 deletions lib/api/src/tests/stories.test.js
Expand Up @@ -205,6 +205,48 @@ describe('stories API', () => {
});
});

it('trims whitespace of group/component names (which originate from the kind)', () => {
const navigate = jest.fn();
const store = createMockStore();

const {
api: { setStories },
} = initStories({ store, navigate, provider });

setStories({
'design-system-some-component--my-story': {
kind: ' Design System / Some Component ', // note the leading/trailing whitespace around each part of the path
name: ' My Story ', // we only trim the path, so this will be kept as-is (it may intentionally have whitespace)
parameters,
path: 'design-system-some-component--my-story',
id: 'design-system-some-component--my-story',
args: {},
},
});

const { storiesHash: storedStoriesHash } = store.getState();

// We need exact key ordering, even if in theory JS doesn't guarantee it
expect(Object.keys(storedStoriesHash)).toEqual([
'design-system',
'design-system-some-component',
'design-system-some-component--my-story',
]);
expect(storedStoriesHash['design-system']).toMatchObject({
isRoot: true,
name: 'Design System', // root name originates from `kind`, so it gets trimmed
});
expect(storedStoriesHash['design-system-some-component']).toMatchObject({
isComponent: true,
name: 'Some Component', // component name originates from `kind`, so it gets trimmed
});
expect(storedStoriesHash['design-system-some-component--my-story']).toMatchObject({
isLeaf: true,
kind: ' Design System / Some Component ', // kind is kept as-is, because it may be used as identifier
name: ' My Story ', // story name is kept as-is, because it's set directly on the story
});
});

it('sets roots when showRoots = true', () => {
const navigate = jest.fn();
const store = createMockStore();
Expand Down
6 changes: 2 additions & 4 deletions lib/client-api/src/story_store.ts
Expand Up @@ -343,10 +343,8 @@ export default class StoryStore {
'Cannot add a story when not configuring, see https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#story-store-immutable-outside-of-configuration'
);

if (storyParameters) {
checkGlobals(storyParameters);
checkStorySort(storyParameters);
}
checkGlobals(storyParameters);
checkStorySort(storyParameters);

const { _stories } = this;

Expand Down

0 comments on commit e530ffb

Please sign in to comment.