Installs Onyx from the react-native-onyx repo#784
Conversation
| [IONKEYS.SESSION]: {loading: false, error: ''}, | ||
| } | ||
| }, | ||
| onStorageEvent: addStorageEventHandler, |
There was a problem hiding this comment.
Some comment about what this does might be useful here.
There was a problem hiding this comment.
I wonder if onStorageEvent is the best name for a method that does both the listening and the handling. I would expect the handler to take an argument of "event" but that's not really what's happening here.
I have two alternative suggestions:
- Change this to
registerStorageEventListenerlike so
registerStorageEventListener: (onStorageEvent) => {
listenToStorageEvents(onStorageEvent);
},- Expose
Ion.keyChanged()so it can be triggered externally without enforcing any opinions about what sorts of things should be able to trigger akeyChanged()event
There was a problem hiding this comment.
Yeah, those are good points. I agree with you. I like that first suggestion best since I think that changes the least about how the lib is supposed to work. I think we can perfect this later if we wish.
|
Merge commit for JS-Libs here: Expensify/expensify-common@92b874e |
marcaaron
changed the title
|
|
marcaaron
left a comment
marcaaron
left a comment
There was a problem hiding this comment.
LGTM just waiting for tests to finish
marcaaron
changed the title
Picks up the sync-throw handling fix in Expensify/react-native-onyx#784 (comment).
3 participants
Held on Expensify/react-native-onyx#1Fixed Issues
Fixes https://github.com/Expensify/Expensify/issues/145302
Tests
npm i && npm run web