Workiva / react-dart

Dart Bindings for React JS
BSD 2-Clause "Simplified" License
412 stars 67 forks source link

FED-1910 Fix react_dom API typings, add tests #382

Closed greglittlefield-wf closed 10 months ago

greglittlefield-wf commented 10 months ago

Motivation

The existing nullability/typings for ReactDom.findDomNode and ReactDom.render exported from package:react/react_client/react_interop.dart are incorrect.

class ReactDom {
  Element findDOMNode(dynamic object) {/*...*/}
  ReactComponent render(ReactElement component, Element element) {/*...*/}
  /*...*/
}

findDOMNode returns null in many cases, but its return type is incorrectly non-nullable.

render returns null for some cases (function components, null), Element for DOM components, and CharacterData for strings and numbers, but is incorrectly typed as non-nullable ReactComponent. The component argument also accepts null and other "ReactNode" arguments to rendered, but its type is incorrectly non-nullable and restricted to just ReactElement.

These bad typings cause runtime errors in some cases. Unfortunately, there wasn't good test coverage around these methods.

These typings also only affect these APIs under the ReactDom class in package:react/react_client/react_interop.dart, and not the top-level Function-typed findDOMNode and render APIs exported from package:react/react_dom.dart, which most consumers use.

Because these typings were incorrect and will lead to runtime errors in some cases, and the changes have a low likelihood of causing breakages, it feels appropriate to release these changes as a patch.

Solution

aviary3-wk commented 10 months ago

Security Insights

No security relevant content was detected by automated scans.

Action Items

greglittlefield-wf commented 10 months ago

@Workiva/release-management-p