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

All parameters are added and merged to all groups + roots in storiesHash #11618

Closed
tmeasday opened this issue Jul 20, 2020 · 1 comment
Closed
Assignees
Milestone

Comments

@tmeasday
Copy link
Member

tmeasday commented Jul 20, 2020

Describe the bug

In #9090 we started storing parameters on all levels of the storiesHash from root down.

The reason for this was to control link behaviour when clicking on the component in the UI (I'm not sure why this meant we started adding parameters for all groups including non-components, given it is behaviour of clicking on components we were concerned about).

This is problematic for two reasons:

  1. There is no such concept as parameters for a (non-component) group or root. In theory we might have one but merging together parameters for kind/components in arbitrary order doesn't make sense if we did. For example if we actually used the parameters on non-component groups this would very likely lead to weird bugs due to ordering (eg. what is viewMode for "Docs" if "Docs/Story" is {viewMode: 'docs'} and "Docs/Table" is {viewMode: 'story'}?) .

  2. This is a big performance problem as we merge together groups many many times and seems likely to be the cause of Storybook >10 seconds to launch, slowed down by merge in transformStoriesRawToStoriesHash #11591

It doesn't look like this code even exists any more although we need to investigate further. @ndelangen you may know more about it (as you refactored the sidebar recently).

@shilman
Copy link
Member

shilman commented Jul 21, 2020

Zoinks!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-rc.13 containing PR #11624 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Jul 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants