Migrate to functional components - 2 #1811
Conversation
…ix/react-native-calendars into infra/more_functional_components
…ix/react-native-calendars into infra/more_functional_components
Inbal-Tish
changed the title
…ix/react-native-calendars into infra/more_functional_components
…nfra/more_functional_components
| } | ||
| const PeriodDay = (props: PeriodDayProps) => { | ||
| const {theme, marking, date, onPress, onLongPress, state, accessibilityLabel, testID, children} = props; | ||
| const dateData = date ? xdateToData(new XDate(date)) : undefined; |
There was a problem hiding this comment.
I noticed this a few times, where you check if a value is undefined before passing it to the a util function.
Consider moving this logic to the util and return undefined if the argument passed is undefined.
It's very similar to how lodash works (they know how to handle undefined parameters)
Also, it makes the code a little cleaner
There was a problem hiding this comment.
Changing the util to return undefined means to change the API for callback props that return this object. We can do it in V2
src/calendar/index.tsx
Outdated
| updateMonth(newMonth); | ||
| }, [currentMonth, updateMonth]); | ||
|
|
||
| const handleDayInteraction = (date: DateData, interaction?: (date: DateData) => void) => { |
There was a problem hiding this comment.
Notice that you might need to wrap this with useCallback.
Usually when one function depends on the other and the one is wrapped the other probably need to be wrapped as well.
Otherwise you'll have issues with old reference of functions being invoked with wrong closure data
There was a problem hiding this comment.
@Inbal-Tish What about this one?
There was a problem hiding this comment.
Missed it. Done
|
@Inbal-Tish |
|
@ethanshar I would like to release this one on this cycle so we can have a full QA before we continue with other components since these are the basics that all the others rely on. Are you waiting for something on my end? |
…nfra/more_functional_components
PeriodDay, Calendar