Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Core: Ensure node name does not contain leading/trailing whitespace #13275

Merged
merged 3 commits into from
Dec 1, 2020
Merged

Core: Ensure node name does not contain leading/trailing whitespace #13275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bf736e2
Trim whitespace in story path parts, to ensure component names will n…
ghengeveld Nov 25, 2020
0f7a3a0
storyParameters will never be falsy because it has a default value.
ghengeveld Nov 25, 2020
627a127
Add test for trimming group/component names.
ghengeveld Nov 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 69 additions & 91 deletions lib/api/src/lib/stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
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),
});
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved

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);
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved
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()] : [];
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved

const rootAndGroups = [...root, ...groups].reduce((list, name, index) => {
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved
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,
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved
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({
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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);
}
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved
checkGlobals(storyParameters);
checkStorySort(storyParameters);

const { _stories } = this;

Expand Down