-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TS migration] Migrate 'useCurrentReportID.js' hook to TypeScript #29264
Changes from 8 commits
e008569
43086a5
38bad6a
54908a6
37e44ef
7107d28
c81cd27
9b9d48e
df83ec2
79a0d2b
79a35c0
f521c2d
8ef5b03
1457851
a207631
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -1,10 +1,18 @@ | ||||||||
import React, {createContext, forwardRef, useCallback, useState, useMemo} from 'react'; | ||||||||
import React, {createContext, forwardRef, useCallback, useState, useMemo, RefAttributes, ComponentType} from 'react'; | ||||||||
import PropTypes from 'prop-types'; | ||||||||
import {NavigationState} from '@react-navigation/native'; | ||||||||
|
||||||||
import getComponentDisplayName from '../libs/getComponentDisplayName'; | ||||||||
import Navigation from '../libs/Navigation/Navigation'; | ||||||||
|
||||||||
const CurrentReportIDContext = createContext(null); | ||||||||
type CurrentReportIDContextValue = { | ||||||||
updateCurrentReportID: (state: NavigationState) => void; | ||||||||
currentReportID: string; | ||||||||
}; | ||||||||
type CurrentReportIDContextProviderProps = { | ||||||||
children: React.ReactNode; | ||||||||
}; | ||||||||
const CurrentReportIDContext = createContext<CurrentReportIDContextValue | null>(null); | ||||||||
|
||||||||
kubabutkiewicz marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
const withCurrentReportIDPropTypes = { | ||||||||
/** Function to update the state */ | ||||||||
|
@@ -18,26 +26,26 @@ const withCurrentReportIDDefaultProps = { | |||||||
currentReportID: '', | ||||||||
}; | ||||||||
|
||||||||
function CurrentReportIDContextProvider(props) { | ||||||||
function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderProps) { | ||||||||
const [currentReportID, setCurrentReportID] = useState(''); | ||||||||
|
||||||||
/** | ||||||||
* This function is used to update the currentReportID | ||||||||
* @param {Object} state root navigation state | ||||||||
* @param state root navigation state | ||||||||
*/ | ||||||||
const updateCurrentReportID = useCallback( | ||||||||
(state) => { | ||||||||
setCurrentReportID(Navigation.getTopmostReportId(state)); | ||||||||
(state: NavigationState) => { | ||||||||
setCurrentReportID(Navigation.getTopmostReportId(state) ?? ''); | ||||||||
}, | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this change needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @situchan So App/src/components/withCurrentReportID.js Lines 20 to 22 in a094cbb
The default state is empty string so useState is infering from this that currentReportID will be always string, and Navigation.getTopmostReportId is returning string | undefined we need to use ?? to put empty string in caseNavigation.getTopmostReportId would return undefined
|
||||||||
[setCurrentReportID], | ||||||||
); | ||||||||
|
||||||||
/** | ||||||||
* The context this component exposes to child components | ||||||||
* @returns {Object} currentReportID to share between central pane and LHN | ||||||||
* @returns currentReportID to share between central pane and LHN | ||||||||
*/ | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
const contextValue = useMemo( | ||||||||
() => ({ | ||||||||
(): CurrentReportIDContextValue => ({ | ||||||||
updateCurrentReportID, | ||||||||
currentReportID, | ||||||||
}), | ||||||||
|
@@ -53,8 +61,8 @@ CurrentReportIDContextProvider.propTypes = { | |||||||
children: PropTypes.node.isRequired, | ||||||||
}; | ||||||||
|
||||||||
export default function withCurrentReportID(WrappedComponent) { | ||||||||
const WithCurrentReportID = forwardRef((props, ref) => ( | ||||||||
export default function withCurrentReportID<TComponentProps extends CurrentReportIDContextValue>(WrappedComponent: ComponentType<TComponentProps>) { | ||||||||
const WithCurrentReportID: ComponentType<TComponentProps & RefAttributes<ComponentType<TComponentProps>>> = forwardRef((props, ref) => ( | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
const X = forwardRef((props: CurrentReportIDContextValue, ref: React.Ref<HTMLInputElement>) => {
console.log(props.currentReportID);
console.log(props.updateCurrentReportID);
return <input ref={ref} />;
});
function Test() {
const Wrapped = withCurrentReportID(X);
const testRef = useRef<HTMLInputElement>(null);
const x = <Wrapped ref={testRef} />;
}
Final code looks like this: export default function withCurrentReportID<TProps extends CurrentReportIDContextValue, TRef>(WrappedComponent: ComponentType<TProps & RefAttributes<TRef>>) {
function WithCurrentReportID(props: Omit<TProps, keyof CurrentReportIDContextValue>, ref: ForwardedRef<TRef>) {
return (
<CurrentReportIDContext.Consumer>
{(currentReportIDUtils) => (
<WrappedComponent
// eslint-disable-next-line react/jsx-props-no-spreading
{...currentReportIDUtils}
// eslint-disable-next-line react/jsx-props-no-spreading
{...(props as TProps)}
ref={ref}
/>
)}
</CurrentReportIDContext.Consumer>
);
}
WithCurrentReportID.displayName = `withCurrentReportID(${getComponentDisplayName(WrappedComponent)})`;
return forwardRef(WithCurrentReportID);
} |
||||||||
<CurrentReportIDContext.Consumer> | ||||||||
kubabutkiewicz marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
{(currentReportIDUtils) => ( | ||||||||
<WrappedComponent | ||||||||
|
@@ -74,3 +82,4 @@ export default function withCurrentReportID(WrappedComponent) { | |||||||
} | ||||||||
|
||||||||
export {withCurrentReportIDPropTypes, withCurrentReportIDDefaultProps, CurrentReportIDContextProvider, CurrentReportIDContext}; | ||||||||
export type {CurrentReportIDContextValue}; |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import {useContext} from 'react'; | ||
import {CurrentReportIDContext, CurrentReportIDContextValue} from '../components/withCurrentReportID'; | ||
|
||
export default function useCurrentReportID(): CurrentReportIDContextValue | null { | ||
return useContext(CurrentReportIDContext); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import {ComponentType} from 'react'; | ||
|
||
/** Returns the display name of a component */ | ||
export default function getComponentDisplayName(component: ComponentType): string { | ||
export default function getComponentDisplayName<TProps>(component: ComponentType<TProps>): string { | ||
return component.displayName ?? component.name ?? 'Component'; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NAB: Copy the comment from propTypes: