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.

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

CSF: Revert makeDisplayName & add stable storyNameFromExport #7901

Merged
merged 6 commits into from Aug 29, 2019

Conversation

shilman
Copy link
Member

@shilman shilman commented Aug 28, 2019

Issue: #7599

What I did

This partially reverts the change made in #7878, keeping the displayName concept, but removing its configurability and adding a new storyNameFromExport concept.

Why I did it

We made hierarchySeparator and hierarchyRootSeparator configurable, and it has been a maintenance nightmare, adding complexity across our codebase and bringing that nightmare to other tools in the Storybook ecosystem.

makeDisplayName is a similar foundational configuration. However since it can contain arbitrary JS code, we expect it will cause an order of magnitude MORE headaches. At the very least we're not ready to sign up for it without a lot more evidence that it's necessary.

This:

  • Improves compatibility with legacy tools that don't support displayName
  • Improves the readability of generated IDs
  • Removes the concept of makeDisplayName entirely, replacing it with a stable storyNameFromExport utility function

Now we transform the CSF named export using lodash.startCase as the default. If you want other behavior you can use the namedExport.story = { name: 'anything goes' } construct.

    it('should format CSF exports with sensible defaults', () => {
      const testCases = {
        name: 'Name',
        someName: 'Some Name',
        someNAME: 'Some NAME',
        some_custom_NAME: 'Some Custom NAME',
        someName1234: 'Some Name 1234',
        someName1_2_3_4: 'Some Name 1 2 3 4',
      };
      Object.entries(testCases).forEach(([key, val]) => expect(storyNameFromExport(key)).toBe(val));
    });

Consider the following CSF:

export default { title: 'comp' };
export const fooBar = () => <>hello</>

Before this change:

storiesOf('comp', module).add('fooBar', () => <>hello</>, { displayName: 'Foo Bar' });

Story ID in URL: comp--foobar

After this change:

storiesOf('comp', module).add('Foo Bar', () => <>hello</>);

Note that if the user manually specifies a display name, it will still get passed through:

fooBar.story = { name: 'whatever' }

=>

storiesOf('comp', module).add('Foo Bar', () => <>hello</>, { displayName: 'whatever' });

Story ID in URL: comp--foo-bar

How to test

cd examples/official-storybook
yarn storybook

@vercel
Copy link

vercel bot commented Aug 28, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@shilman shilman added this to the 5.2.0 milestone Aug 28, 2019
@shilman shilman added csf maintenance User-facing maintenance tasks labels Aug 28, 2019
@tmeasday
Copy link
Member

I think this is good if we commit to the current version of makeDisplayName and don't change it in the future.

Otherwise having ids change between versions of storybook is a hot mess I guess ;)

Definitely makes more sense to do this if the user can't change that function.

@vercel vercel bot temporarily deployed to staging August 29, 2019 01:43 Inactive
@shilman
Copy link
Member Author

shilman commented Aug 29, 2019

@tmeasday We can make this stable AND potentially configurable in the future:

storiesOf('comp', module).add('Foo Bar', () => <>hello</>, { displayName: userFn('fooBar') });

@tmeasday
Copy link
Member

I guess although that could be pretty confusing too

@shilman
Copy link
Member Author

shilman commented Aug 29, 2019

@tmeasday I've updated the code to try to make it less confusing & updated the description up top.

@shilman shilman changed the title CSF: Run display ids through makeDisplayName heuristic CSF: Revert makeDisplayName config and add stable storyNameFromExport function Aug 29, 2019
@shilman shilman changed the title CSF: Revert makeDisplayName config and add stable storyNameFromExport function CSF: Revert makeDisplayName & add stable storyNameFromExport Aug 29, 2019
@shilman shilman merged commit aca513d into next Aug 29, 2019
@shilman shilman deleted the 7599-csf-display-ids branch August 29, 2019 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
csf maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants