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

Refactor story state mechanism #9811

Closed
tmeasday opened this issue Feb 11, 2020 · 7 comments
Closed

Refactor story state mechanism #9811

tmeasday opened this issue Feb 11, 2020 · 7 comments

Comments

@tmeasday
Copy link
Member

We have useStoryState, however it is buggy and not very well tested.

Implement a new state field on story (eq. parameters) that:

  • is mutable, both from the manager and preview,
  • the "master" copy is stored in the store
  • whenever it changes, it re-renders the current story.

Additionally:

  • refactor useStoryState to leverage it, manager+preview,
  • add a "state initializer" concept to the store that allows you to initialize the state of a story before it renders, based on parameters.
@atanasster
Copy link
Member

That's the spirit - I think I have seen that approach somewhere :)
A few notes:

  • Unless I misunderstand, but there isn't anything to refactor from useStoryState (except the name), right? The approach will be to implement the state in the api and then if users do need a hook, create a useStoryState hook on top of the api.
  • "initialize the state of a story before it renders" - that will be a bit of a hack and in my opinion will cause issues long term, I believe it will be better to initialize state at story "create" (loading of stories)

Good luck.

@tmeasday
Copy link
Member Author

tmeasday commented Feb 11, 2020

Unless I misunderstand, but there isn't anything to refactor from useStoryState (except the name), right? The approach will be to implement the state in the api and then if users do need a hook, create a useStoryState hook on top of the api.

Heh, I guess it just depends what you mean by "refactor" ;)

"initialize the state of a story before it renders" - that will be a bit of a hack and in my opinion will cause issues long term, I believe it will be better to initialize state at story "create" (loading of stories)

Yeah I think I agree with this part.

@atanasster
Copy link
Member

Lol :)

@stale
Copy link

stale bot commented Mar 3, 2020

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Mar 3, 2020
@shilman
Copy link
Member

shilman commented Mar 10, 2020

Egads!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-alpha.22 containing PR #10014 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 Mar 10, 2020
@stale stale bot removed the inactive label Mar 10, 2020
@shilman shilman reopened this Mar 10, 2020
@tmeasday
Copy link
Member Author

We still need to delete useStoryState as per a conversation with @ndelangen

@shilman
Copy link
Member

shilman commented Mar 11, 2020

Whoopee!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-alpha.24 containing PR #10015 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 Mar 11, 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

3 participants