FED-1979 Add suggestor to replace null argument in dom callback

[aaronlademann-wf]

Motivation

Changes

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 following commands on any of the repos in this SG query locally

    dart pub global activate https://github.com/Workiva/over_react_codemod.git --source=git --git-ref=batch/fedx/FED-1979_dom_callback_null_args
    dart pub global run over_react_codemod:dom_callback_null_args --yes-to-all
    

    and verify that the dom callbacks that used to pass null literal as an argument, now have the correct empty synthetic event being passed instead.

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

[aviary2-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.

[annawatson-wk]

+10,

Excluded React DOM event callback props aren't used in the Workiva ecosystem in test files and non-test files:
sourcegraph
sourcegraph

Great test coverage!

Ran in w_comments and it updated as expected.

[sydneyjodon-wk]

I haven't reviewed this PR, but I have the same question on all these - do we want to have just one null_safety_prep that all these codemods can go into instead of adding a bunch of separate codemods?

[aaronlademann-wf]

@sydneyjodon-wk I was wondering about that actually - I think thats a good idea...

Should I remove the individual bin/ executables here and in #273? Or can we add an aggregated executable in a follow-up PR without modifying these?

[sydneyjodon-wk]

Is there a reason we would need separate executables along with the aggregated one?

[aaronlademann-wf]

Not necessarily - was just thinking we could create the aggregate executable in a single pr rather than having to push changes to two existing PRs

[sydneyjodon-wk]

oh yeah that works! I think we should remove the individual executables from both PRs though if we don't think they're needed if that's what you were asking!

[aaronlademann-wf]

@sydneyjodon-wk done (in both PRs)

[annawatson-wk]

+10, reup - just removed standalone executable

[sydneyjodon-wk]

This looks great, just had the same comments as the other one!

[sydneyjodon-wk]

Wondering if this one doesn't need to be resolved either?

[sydneyjodon-wk]

Are the values in this map just for testing purposes? I only see the keys being used in this file?

[sydneyjodon-wk]

I think this file can be removed too in favor of adding this to the null_safety_prep codemod

[aaronlademann-wf]

@sydneyjodon-wk I've made the necessary updates to get this running as part of the larger null_safety_prep executable. Should be ready to go now.

[sydneyjodon-wk]

+10 tested on web_skin_dart and Anna tested on w_comments as well! 🎉

@Workiva/release-management-p

[rmconsole-wf]

+1 from RM