[$500] DEV: Bank Account - Console error on Connect bank account page

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Go to settings > Workspaces
2. Go to any policy and click on bank account

Expected Result:

It should not show any console error

Actual Result:

console error appears (forwardRef render functions accept exactly two parameters: props and ref. Did you forget to use the ref parameter?)

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: Dev v1.3.74-2
Reproducible in staging?: n
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-09-27.at.1.41.17.PM.1.mov

Screen.Recording.2023-09-27.at.1.38.55.PM.1.mov

Expensify/Expensify Issue URL:
Issue reported by: @gadhiyamanan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695802455045679

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016340d5d733591aa3
• Upwork Job ID: 1707392958356664320
• Last Price Increase: 2023-09-28
• Automatic offers:
• aimane-chnaif | Reviewer | 27009949
• gadhiyamanan | Reporter | 27009950

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~016340d5d733591aa3

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @aimane-chnaif (External)

[ikevin127]

Proposal

Please re-state the problem that we are trying to solve in this issue

Console error appears (forwardRef render functions accept exactly two parameters: props and ref. Did you forget to use the ref parameter?).

What is the root cause of that problem?

The problem stems from the RequestorStep.js component export which does not forward the props / ref of the forwardRef function like other components do.

Current implementation:

export default React.forwardRef(RequestorStep);

Other components example:

export default React.forwardRef((props, ref) => (
    <AmountTextInput
        // eslint-disable-next-line react/jsx-props-no-spreading
        {...props}
        forwardedRef={ref}
    />
));

What changes do you think we should make in order to solve the problem?

Would forward the ref correctly in order to get rid of the console error, applying the following changes within the RequestorStep.js:

- export default React.forwardRef(RequestorStep);

+export default React.forwardRef((props, ref) => (
+    <RequestorStep
+        // eslint-disable-next-line react/jsx-props-no-spreading
+        {...props}
+        forwardedRef={ref}
+    />
+));

What alternative solutions did you explore? (Optional)

N/A

Videos

web.mp4

[getusha]

@ikevin127 i think this pattern was intentional please check here https://expensify.slack.com/archives/C01GTK53T8Q/p1695110739750909

[pradeepmdk]

Proposal

Please re-state the problem that we are trying to solve in this issue.

console error on RequestorStep

What is the root cause of that problem?

in ReimbursementAccountPage we are passing the ref to the child but RequestorStep forwarding the ref but we never used that

App/src/pages/ReimbursementAccount/RequestorStep.js

Line 75 in bfda42d

function RequestorStep({reimbursementAccount, shouldShowOnfido, reimbursementAccountDraft, onBackButtonPress, getDefaultStateForField}) {

What changes do you think we should make in order to solve the problem?

1 option

just remove the
React.forwardRef(RequestorStep) => RequestorStep
and in ReimbursementAccountPage need to remove the this.requestorStepRef Currently this will no ref have because
RequestorStep does not forward any ref.

from this PR https://github.com/Expensify/App/pull/27455/files

2 options

we need to change this

const  RequestorStep = React.forwardRef(({reimbursementAccount, shouldShowOnfido, reimbursementAccountDraft, onBackButtonPress, getDefaultStateForField}, ref) => {

++
 <RequestorOnfidoStep
                ref={ref}
++
<ScreenWrapper
            includeSafeAreaPaddingBottom={false}
            ref={ref}

++
RequestorStep.propTypes = propTypes;
RequestorStep.displayName = 'RequestorStep';

export default RequestorStep;

if we change this we need to properly handle ref in the child component as well.

this will cover this issue as well
forwardRef render functions do not support propTypes or defaultProps. Did you accidentally pass a React component?

[Pujan92]

Seems a regression from #27455
@kacper-mikolajczak can you plz confirm

[aimane-chnaif]

yes regression from #27455 but intentional
https://expensify.slack.com/archives/C01GTK53T8Q/p1695110739750909

[kacper-mikolajczak]

Hi @Pujan92 @aimane-chnaif, I can confirm the regression stems from changes to RequestorStep.js. Thanks to all of you for noticing that and proposing fixes 👍

Let me know if I should handle that - I am happy to help :) Down below I posted my proposal.
@pradeepmdk's second option looks great and it is very similar to what I would suggest as a fix here.

P.S.
Here is a link to the updated docs.

[kacper-mikolajczak]

Please re-state the problem that we are trying to solve in this issue.

Error regarding ForwardRef props is posted to the console when going through Bank account flow. Additionally, warning about propTypes is also present in the console.

What is the root cause of that problem?

RequestorStep component is missing ref.

What changes do you think we should make in order to solve the problem?

We need to properly pass forwarded ref inside a component by anchoring it to children components.

With usage of propTypes we can't use current convention and it looks like the best approach here is to inline the component definition inside forwardRef so warning disappears.

[aimane-chnaif]

As it's from @kacper-mikolajczak's PR, I agree @kacper-mikolajczak handle this.
🎀 👀 🎀 C+ reviewed