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

addon API for storing state per-session or per-app. #5271

Closed
tmeasday opened this issue Jan 16, 2019 · 6 comments
Closed

addon API for storing state per-session or per-app. #5271

tmeasday opened this issue Jan 16, 2019 · 6 comments

Comments

@tmeasday
Copy link
Member

tmeasday commented Jan 16, 2019

Issue: we removed the use of URL params for storing state. This will be an issue if apps reload (e.g. because HMR isn't working right, see for instance #5267).

Right now we have an adhoc way of using session/localStorage for storing things. We also have retained setQueryParams but it no longer actually affects the URL so will not survive reloads etc.

We should resolve this for v5.

@tmeasday
Copy link
Member Author

tmeasday commented Jan 17, 2019

Here's what I sketched out in my head:

What if we update setState to have the signature:

function setState(update: object, persistence: 'none' | 'session' | 'forever') {}

When we setState, we may also save a "subset" of the state into sessionStorage and localStorage based on the above.

When we initialize the state, we will combine:

  1. The default initial state returned by each module
  2. The forever state
  3. The session state

Merging them with preference in that order (i.e. session > forever > defaults)

@shilman
Copy link
Member

shilman commented Jan 23, 2019

w00t!! I just released https://github.com/storybooks/storybook/releases/tag/v5.0.0-alpha.9 containing PR #5289 that references this issue. Upgrade today to try it out!

Because it's a pre-release you can find it 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 Jan 23, 2019
@issue-sh issue-sh bot removed the high priority label Jan 23, 2019
@leoyli
Copy link
Contributor

leoyli commented Feb 7, 2019

@shilman, this is not working in the beta.1 version. (not sure about the prior versions)

For example, in a decorator I call api.setQueryParams({ test: 'abc' }). The URL is not changed and the subsequent call both (console.dir(api.getUrlState()) and console.dir(api.getQueryParam())) was not showing the query parameter I provided...

Any clue or should I open a new issue?!

@tmeasday
Copy link
Member Author

tmeasday commented Feb 7, 2019

@leoyli -- I think open a new issue. This issue was specifically about an API to replace the setQueryParams API, but we are aiming for back-compat there, so if it is broken, it is worth fixing.

@leoyli
Copy link
Contributor

leoyli commented Feb 8, 2019

I see, I will take a time to gather enough information and try to reproduce it using some sandbox or in a new project. After that I can open an issue! Thanks!

@andyindahouse
Copy link

andyindahouse commented Mar 13, 2019

any news with this?

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

4 participants