React-native: Require user-provided async storage #7801
Conversation
Gongreg
requested review from
danielduan,
rmevans9 and
tmeasday
as code owners
|
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/storybook/monorepo/m31knmawo |
|
@shilman, it is kinda a breaking change, we are making some functionality opt in. |
|
@Gongreg can you give it a default value that does not make it a breaking change? |
|
Not really, since that is the main reason why I move async storage out. I don’t know what is going to be used. If user doesn’t pass anything nothing bad will happen. It will simply skip trying to read/write to asyncStorage. Thats why it is not really a breaking change, but rather moving out feature as opt in. |
|
why can't you just make it default to the previous behavior, then it will not be breaking at all. and users can still overwrite it to use a different storage provider if they wish. in 6.0 you can make it completely opt in? |
|
or does simply having the dependency introduce problems? |
|
Exactly. If we introduce new dependency it brings native dependencies and we cant use old one since it doesnt exist in RN 59+ |
|
@shilman, so what should we do next? |
|
@Gongreg we could default to |
|
I guess we can do that for deprecation. But we also cant start to use react native community one. |
|
Right. After the deprecation, the default implementation will be a noop |
|
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! |
|
Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook! |
|
@shilman, @ndelangen I still feel like this is a way to solve the issue. This will not be a "real" breaking change. If users want to continue to use asyncStorage then they will have to update, but they won't have to do anything if they don't need it. |
|
There is already React Native version 0.61, so I think we are blocking users from using storybook by not implementing this fix. |
|
@Gongreg please document the migration and then we break semver for this
|
|
@shilman, regarding the warning, you mean that in version v5 we display a warning to use new key, or manually set it to false? And in v6 we just remove the warning? |
|
@shilman, added a warning |
shilman
changed the title
Issue: #6078
What I did
Removed async storage usage in storybook, added ability to pass custom storage.
react-native-community/async-storage is a native dependency that requires linking. Some apps might not use it, so that would mean that storybook requires native dependency that the app itself doesn't have. That bring additional barrier to users. To avoid it we can allow to pass asyncStorage as a parameter to getStorybookUI({asyncStorage: require('react-native').AsyncStorage or require('@react-native-community/async-storage') or any other storage that has getItem and setItem}). Overall async storage functionality in storybook itself is not a core feature but a good to have thing, so asking for additional option shouldn't be too bad.