-
-
Notifications
You must be signed in to change notification settings - Fork 145
Conversation
cc @nicolaskruchten - I know you have were thinking about a dcc.Store component. |
Yep, although I was mostly concerned about security: plotly/dash#262 |
I think this is a great idea, even without server-side encryption this would be a nice improvement over the current hidden div hack. Is there a way to persist this data between different sessions? Edit: sorry, I see this would be the |
Also, this seems orthogonal to the issue of whether or not to use e.g. Redis too. A more comprehensive solution using something like Redis to store user data would still be useful if there's large user-specific data that only needs to exist server-side, since it would reduce traffic needed to pass data between components, but that obviously requires a lot of extra set-up. Being able to use HTML localStorage with 'off-the-shelf' Dash would be really nice in my opinion, and be sufficient for most people (myself included). |
This seems a lot cleaner than the hidden div hack, and I think it is useful enough to release without encryption and then add encryption as a separate PR, as long as we make it clear in the docs the stored data is available to the client |
This looks good to me. My main feedback is about the name. I wonder if we should be clear that this is client-side storage. Some different name ideas:
I think I prefer I would also like @ned2 's opinion on this. Otherwise, I'm 💃 on this in principle. I haven't taken a look into the code |
I'd also like to play around with this a little bit actually. It's not clear to me how this would fit into a set of callbacks. i.e. how would you structure your code so that the backend knows whether data is already available in the store or it needs to be added? perhaps we could add a lightweight "
|
That is one of the issue I had, I think adding a timestamp should help, I'll try that.
The api for the local and session is called I prefer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Got a couple of comments though. I'm also wondering why you would need a timestamp
or anything as such - wouldn't you just be able to compare the data coming from props
to the storage, and update accordingly?
src/components/Storage.react.js
Outdated
|
||
class MemStore { | ||
constructor() { | ||
this._data = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering about why you're not using React's state
here, as in this.state = {data:{}}
? Using React's state
and methods like setState
would be more clear to React programmers, and then there's no need to write class methods like getItem
etc.
src/components/Storage.react.js
Outdated
} | ||
} | ||
|
||
const _localStore = new WebStore(window.localStorage); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, wouldn't it be better to define these on the component itself, i.e. Storage
? Although a WebStore
class is nice, I think it gets a little bit too convoluted for React - one of the nice things of React is writing all your code up in components.
src/components/Storage.react.js
Outdated
super(props); | ||
|
||
if (props.storage_type === 'local') { | ||
this._backstore = _localStore; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're already setting it here, couldn't you just use localStorage
here (or window.localStorage
, although I think window
isn't needed) and use the API as is? i.e. localStorage.setItem()
and getItem()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also wrap getItem
with JSON.parse
and and setItem
with JSON.stringify
.
4df22ee
to
bff4bcf
Compare
@plotly/dash Ready for another round of reviews, I added a timestamp for when the data is modified. Available in version 0.30.0rc1. I made a poll for the component name: https://strawpoll.com/ed9ychp8 |
da5bbf3
to
beaef00
Compare
Renaming the storage component to
|
b3e5af7
to
ed951fb
Compare
ed951fb
to
98feaa4
Compare
dash-core-components==0.30.0rc1 seems to hide my graph's axis labels somehow. If I switch to the latest version of dcc, the axis titles (labels) are visible but the dcc.Storage component isn't available in dcc==0.43.0. If I move to dcc==0.30.0rc1, Storage is available but the axis labels on my graph disappear. Are there any syntax incompatibility issues with the following code: `layout = go.Layout(
|
Why is |
@plopd I think it was renamed to |
Available in dash-core-components==0.30.0rc1
pip install dash-core-components==0.30.0rc1
Store component
Alternative to the hidden div method for caching data on the front-end, no need to
json.dumps
andjson.loads
.Three type of storage (
storage_type
prop):memory
- Data is only kept in memory as prop by the component, cleared on refresh.local
- Data is kept inwindow.localStorage
, only cleared manually.session
- Data is kept inwindow.sessionStorage
, cleared on browser exit.** Data must be json serializable. **
Example:
cc @plotly/dash
Closes #229