CPLAT-13185 Fix JsBackedMap for Dart 2.12

[sydneyjodon-wk]

Motivation

Dart 2.12 includes changes to setProperty that disallow passing in Dart functions, since they are not callable by JS code. https://github.com/dart-lang/sdk/commit/d683f33f5bfefe92ea1a590a800c7e41b213f69c

This breaks passing function props to Dart components, since JsBackedMap stores those values using setProperty under the hood.

  Assertion failed: org-dartlang-sdk:///lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart:166:7
  _isJsObject(f) ||
            !JS<bool>('bool', '# instanceof #.Function', f, global_)
  "Dart function requires `allowInterop` to be passed to JavaScript."
  dart:sdk_internal                                        setProperty
  package:react/react_client/js_backed_map.dart 66:5       _set
  dart:sdk_internal                                        addAll
  package:react/react_client/js_backed_map.dart 96:13      addAll
  package:react/react_client/js_backed_map.dart 34:61      <fn>
  package:react/react_client/js_backed_map.dart 34:73      from
  package:react/src/react_client/factory_util.dart 167:34  generateJsProps
  package:react/react_client/component_factory.dart 86:7   generateExtendedJsProps

This is the reason that react-dart dev builds are currently failing.

We can work around this issue by wrapping Dart functions in an object, which can be passed through without issue.

• This shouldn't affect behavior
  • On the JS side, since JS code should already be treating Dart functions as non-callable, and any functions meant to be called by the JS are properly converted first
  • On the Dart side, since nothing accesses the raw JS values in the first place

Changes

• Added workaround based on Greg's Partial implementaion: https://github.com/Workiva/react-dart/pull/298/commits/371f5f7735d8475daafff988826cc9e4aa6c6889
  • Applied the same workaround from JsBackedMap to ContextHelpers
• Tests already cover both allowInterop-wrapped and non-allowInterop-wrapped functions in JsBackedMap and context tests
• Ensure Dart dev channel builds are passing
• Perform consumer testing in over_react, WSD, and major Wdesk repos (DPC, graph_app, home)
  • Since these change affects such a core part of React interop, we want to make sure we're confident it won't cause regressions

QA Steps

• [x] CI passes (including dev channel builds)
• [x] Verify tests adequately cover allowInterop-wrapped and non-allowInterop-wrapped functions (see shared_type_tester.dart)
• [x] Verify that consumer tests are passing in:
  • [x] over_react (except dev channels because of NNBD)
  • [x] web_skin_dart
  • [x] doc_plat_client
  • [x] cerebral-ui
  • [x] graph_app
  • [x] home

[aviary3-wk]

Security Insights

The items listed below may not capture all security relevant changes. Before providing a security review, be sure to review the entire PR for security impact.

(1) Security relevant changes were detected

Watched file lib/src/react_client/private_utils.dart modified

Action Items

• Obtain a security review; reviewer should pay special attention to insights listed above

• Verify aviary.yaml coverage of security relevant code

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

[semveraudit-wf]

Public API Changes

No changes to the public API found for commit 32b58a7a363d7e9b944b8f5984c0243c04a7cdeb

Showing results for 32b58a7a363d7e9b944b8f5984c0243c04a7cdeb

Powered by semver-audit-service. Please report any problems by filing an issue. Reported by the dart semver audit client 2.2.0 Browse public API.

Last edited UTC Feb 17 at 19:59:18

[evanweible-wf]

+1 security