refactor: convert to functional component - #124
Conversation
| if (results.access_token) { | ||
| // Keeping this to keep it backwards compatible, but also returning raw results to account for future changes. | ||
| this.props.onLoginSuccess(results.access_token, results); | ||
| onLoginSuccess(results.access_token, results); |
There was a problem hiding this comment.
should we assume that onLoginSuccess is required every time?
There was a problem hiding this comment.
add a conditional to validate that onLoginSuccess exists
| onLoginSuccess(results.access_token, results); | |
| if (onLoginSuccess) { | |
| onLoginSuccess(results.access_token, results); | |
| } |
| if (responseType === 'code' && !appSecret) { | ||
| if (code) { | ||
| this.props.onLoginSuccess(code, results); | ||
| onLoginSuccess(code, results); |
| onLoginSuccess(code, results); | ||
| } else { | ||
| this.props.onLoginFailure(results); | ||
| onLoginFailure(results); |
|
|
||
| if (res) { | ||
| this.props.onLoginSuccess(res.data, results); | ||
| onLoginSuccess(res.data, results); |
| onLoginSuccess(res.data, results); | ||
| } else { | ||
| this.props.onLoginFailure(results); | ||
| onLoginFailure(results); |
| this.hide(); | ||
| this.props.onLoginFailure(json); | ||
| hide(); | ||
| onLoginFailure(json); |
| webViewState.url === 'https://www.instagram.com/' | ||
| ) { | ||
| this.setState({ key: key + 1 }); | ||
| setKey((k) => k + 1); |
There was a problem hiding this comment.
why exactly do you need to increase the key counter on that? just to force a rerender?
There was a problem hiding this comment.
I didn't understand too
Should we do this change in this PR, since its only goal is to move from class to FC?
|
I think this refactor may affect consumers that call the public imperative API through a ref: After converting the component to a function component, these methods need to be explicitly exposed. For React 18 and earlier, this requires React 19 supports receiving Since the current class component is stable and the migration does not add a user-facing benefit, I think we should avoid merging this refactor for now unless it preserves the existing |
3 participants
Most react code written today uses functional components and hooks, with this PR, the main class component (
Instagram) is converted to its equivalent functional component.It does not lead to changes in behavior or improvements in performance, but it brings the code closer to the present day and will facilitate future maintenance by other people.
renderCloseandrenderWebvieware still inner functions of the main component