FED-1889 Null Safety: Audit public utility components

[aaronlademann-wf]

• Prop nullability, upgrade props/state to late for public components, update class component prop defaulting
  • ErrorBoundary
  • ResizeSensor
  • AbstractTransition
  • WithTransition
  • FluxUiComponent
  • ConnectFluxPropsMixin
  • ReduxProviderPropsMixin
  • ReduxMultiProvider (storesByContext)

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

[greglittlefield-wf]

Just two small comments, one of em a question. This looks great!

[greglittlefield-wf]

Hmmm thinking it a little more, making actions required might be a little tricky.

While it'll be present in most cases, there are usages that only wire up a component to redraw on the store, because the actions aren't used. Internally, it looks like ~5% of flux component usages had a store with no actions.

So our options are either:

• Make it optional
• Make it required, and plan on migrating consumers to also set ..actions = (either to an actions class that will be unused, or to set ..actions = null and potentially update the TActions generic of the consuming component to be with FluxUiPropsMixin<MyStore, Null>)

5% isn't a lot, so I'm leaning toward making it required, but it'll be a little bit of a pain to update those usages.

Thoughts?

[aaronlademann-wf]

I like making it required.

[greglittlefield-wf]

Looks like there's a failing test:

❌ [Chrome, Dart2Js] test/over_react_redux_test.dart: ReduxMultiProvider works as expected when storesByContext is null (failed)
  Expected: emits the propType warning some element contains 'RequiredPropError: Prop ReduxMultiProviderProps.storesByContext is required.'
    Actual: <Closure: () => TestJacket<Component> from: () => {
                        let t9;
                        return jacket$.mount(react.Component, redux_multi_provider.ReduxMultiProvider().call((t9 = dom_components.Dom.div(), (() => {
                          t9.addTestId("content");
                          return t9;
                        })()).call("foo")));
                      }>
     Which: has propType warning with value [
              'Warning: Failed prop type: Expected a value of type \'Map<Context<dynamic>, Store<dynamic>>\', but got one of type \'Null\''
            ]

That test should probably just be removed, since when we add required prop validation it'll fail for that case when invoking the builder.

[aaronlademann-wf]

Removed the test in 136eb83

[greglittlefield-wf]

Changes look good, +10 assuming CI passes