BREAKING CHANGE: storage migration #11
Conversation
nachomazzara
force-pushed
the
feat/storage-migration
branch
from
ef040dd to
a3e6bcf
Compare
nachomazzara
changed the title
src/lib/localStorage.ts
Outdated
| const newData = migrations[version](data) | ||
| localStorage.setItem( | ||
| key, | ||
| JSON.stringify({ ...(newData as Object), storage: { version } }) |
There was a problem hiding this comment.
it looks like this line is getting rid of the loading: true property that comes with the initial state of the storage reducer (see this line https://github.com/decentraland/decentraland-dapps/pull/11/files#diff-8544c74e5fd9e4b18f35209ce5efd2beR12) I dunno if that breaks anything, probably @nicosantangelo knows better than myself about this, but it seems like this should rather be:
JSON.stringify({ ...(newData as Object), storage: { ...newData.storage, version } })
There was a problem hiding this comment.
I think this is not a big deal because the loading will typically be set after creating the middleware, as a part of the redux-storage lib.
That said I think it doesn't hurt adding the spread, just in case
Also, can we get rid of the as Object? Maybe be a bit more specific?
There was a problem hiding this comment.
@nicosantangelo any is ok? If we want something more specific we need to pass the type when calling the method. For the built-in localstorage data will be defined internally inside decentraland-dapps but for new types, will need to be defined outside
@cazala I guess we are not picking the loading property from the store (I could check it again). We only use version https://github.com/decentraland/decentraland-dapps/pull/11/files#diff-dc8e3a1c851df38886297332fa450dc2R40
|
I left a question in the comments but it looks pretty good to me 👍 |
nachomazzara
force-pushed
the
feat/storage-migration
branch
from
ed570d0 to
a49f9e3
Compare
README.md
Outdated
| storageKey: 'storage-key' // this is the key used to save the state in localStorage (required) | ||
| paths: [] // array of paths from state to be persisted (optional) | ||
| actions: [] // array of actions types that will trigger a SAVE (optional) | ||
| migrations: migrations // migration object that will migrate your localstorage |
There was a problem hiding this comment.
this should be marked as (optional) too
README.md
Outdated
| The storage module allows you to save parts of the redux store in localStorage to make them persistent. | ||
| This module is required to use other modules like `Transaction` and `Translation`. | ||
| The storage module allows you to save parts of the redux store in localStorage to make them persistent and migrate it from different versions without loosing it. | ||
| This module is required to use other modules like `Transaction`, `Translation`, `Wallet` and `Storage`. |
There was a problem hiding this comment.
The Storage module is irrelevant in this enumeration (this is already the Storage module)
README.md
Outdated
|
|
||
| ```ts | ||
| export const migrations = { | ||
| 1: migrateToVersion1(data), |
There was a problem hiding this comment.
Does the 1 migration ever exist? doesn't it start from 2?
There was a problem hiding this comment.
Right, good catch!
README.md
Outdated
| @@ -347,7 +373,7 @@ export const rootReducer = storageReducerWrapper( | |||
|
|
|||
| ### Advanced Usage | |||
|
|
|||
| This module is necessary to use other modules like `Transaction` or `Translation`, but you can also use it to make other parts of your dApp's state persistent | |||
| This module is necessary to use other modules like `Transaction`, `Translation`, `Wallet` and `Storage`, but you can also use it to make other parts of your dApp's state persistent | |||
There was a problem hiding this comment.
Same as the comment above #11 (comment)
BREAKING CHANGE: storage migrations
BREAKING CHANGE: storage migrations
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# Notes We need first to make a release merging decentraland/decentraland-dapps#11
close #10
Add migrations for localstorage keys