FED-1718 Callback ref null safety hint suggestor

[sydneyjodon-wk]

Motivation

In React, callback refs are called with the component instance upon render, and can be called with null when the component unmounts or when the component providing the ref rerenders.

For example instance:

// @Dart=2.11
..ref = (FooComponent ref) {
// ref could be null!
_myRefVariable = ref;
}
As a result, callback refs should be typed as if the ref passed to them could be null.

The null safety migrator tool, however, seems to like to make these refs non-nullable.

To help with nullability inference during the null safety migration of over_react components, and to help prevent null errors afterwards, we need to develop codemods that adjust the typings of over_react callback refs.

Changes

• Add a suggestor that changes the following cases:

1. For ref prop param types:

- (ButtonToolbar()..ref = (ButtonElement r) => ref = r)();
+ (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)();

2. For ref variable declarations:

- ButtonElement ref;
+ ButtonElement /*?*/ ref;
(ButtonToolbar()..ref = (r) => ref = r)();

3. For ref prop type casts:

- (ButtonToolbar()..ref = (r) => ref = r as ButtonElement)();
+ (ButtonToolbar()..ref = (r) => ref = r as ButtonElement /*?*/)();

Release Notes

Review

See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.

Please review:

QA Checklist

• Tests were updated and provide good coverage of the changeset and other affected code
• Manual testing was performed if needed
  • Steps from PR author:
    • Run the codemod on some consuming repos using:

dart pub global activate https://github.com/Workiva/over_react_codemod.git --source=git --git-ref=callback-ref-null-safety-hint
dart pub global run over_react_codemod:null_safety_prep --yes-to-all

- [ ] Anything falling under manual testing criteria [outlined in CONTRIBUTING.md][contributing-manual-testing]

Merge Checklist

While we perform many automated checks before auto-merging, some manual checks are needed:

• A Frontend Frameworks Design member has reviewed these changes
• There are no unaddressed comments - this check can be automated if reviewers use the "Request Changes" feature
• For release PRs - Version metadata in Rosie comment is correct

[aviary-wf]

Security Insights

No security relevant content was detected by automated scans.

Action Items

• Review PR for security impact; comment "security review required" if needed or unsure
• Verify aviary.yaml coverage of security relevant code

---

Questions or Comments? Reach out on Slack: #support-infosec.

[aaronlademann-wf]

When I ran this in admin_client, the _hintAlreadyExists() fn didn't seem to work in all cases - or at least that is what seems like is happening, because I see this in some changed files:

[aaronlademann-wf]

Should we make this something that doesn't produce stdout by default? Pretty noisy...

[sydneyjodon-wk]

@aaronlademann-wf yeahhh I was talking to Greg about that edge case because if the ref is referenced multiple times it will keep adding the nullability hint and we decided that it wouldn't be worth the effort to make the codemod be internally aware of the changes it was making to reduce duplication since it's an easy manual fix and I think the migrator tool should work regardless of comment duplication

[aaronlademann-wf]

+1 / +10

Ran the codemod locally in admin_client, xbrl-module, w_table, graph_ui and doc_plat_client. All expected changes look good, no unexpected changes or rtes while executing the codemod.

[sydneyjodon-wk]

@Workiva/release-management-p

[rmconsole-wf]

+1 from RM