Closed fabioh8010 closed 4 months ago
@roryabraham @blazejkustra I've also upgraded both Onyx and E/App TS to 5.4.5 to be able to use NoInfer type, necessary to fix initialValue
issues with TS.
Putting this on HOLD because I'm going to be OOO next week, and will return on May 6th.
@roryabraham @techievivek I'm taking this PR off the hold.
🚀Published to npm in v2.0.37
Details
This PR does the following changes:
useOnyx
to returnundefined
instead ofnull
, thus allowing us to use default values more easily.OnyxEntry
andOnyxCollection
types to allowundefined
values (necessary because of the change above).Onyx.set
andOnyx.merge
types to disallow passingundefined
directly (like it was before).Onyx.connect
callback's value type to excludeundefined
(undefined
will never be passed).withOnyx
.useOnyx
return read-only value to avoid future situations like this one (it's not committed yet but the problem was because an Onyx value was being mutated during runtime, which causesgetSnapshot()
to always fail the deep-equality check and re-render infinitely).initialValue
type to avoid inferuseOnyx
's return valueinitialValue
.⚠️ Follow-up PR on E/App when these changes are available there: https://github.com/Expensify/App/pull/40367
⚠️ Playground branch: https://github.com/fabioh8010/expensify-app/tree/feature/playground-use-onyx
Related Issues
$ https://github.com/Expensify/App/issues/34339
Automated Tests
Manual Tests
Some common tasks on E/App:
useOnyx
).useOnyx
).useOnyx
).Author Checklist
### Related Issues
section aboveTests
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
I'm having issue to build Android app.Android: mWeb Chrome
https://github.com/Expensify/react-native-onyx/assets/20051562/b6c57db7-c4f2-42fb-ac4c-88d14ed10b29iOS: Native
It's failing runtime due to changes on this repo's `main`. ![Simulator Screenshot - iPhone 15 Pro Max - 2024-04-26 at 15 52 16](https://github.com/Expensify/react-native-onyx/assets/20051562/6f7857de-37bf-4e20-ae53-af5aa99f7d50)iOS: mWeb Safari
https://github.com/Expensify/react-native-onyx/assets/20051562/f8f779c4-7c7e-4589-8925-cd342327e6d9MacOS: Chrome / Safari
https://github.com/Expensify/react-native-onyx/assets/20051562/97460201-260d-42aa-aebc-1a78f5f931bf https://github.com/Expensify/react-native-onyx/assets/20051562/7b8aadf1-11fc-47f4-8b43-798541542887MacOS: Desktop
https://github.com/Expensify/react-native-onyx/assets/20051562/d67d7f58-a492-42cc-8258-4f7144757aa9