FED-3585 Fix fragment type check assert

[greglittlefield-wf]

Motivation

getProps(child, traverseWrappers: true) incorrectly raises the following assertion error, only in dart2js (when using --enable-asserts):

Assertion failed: "`type` should be a valid component type (and not null or a type alias)."
/_internal/js_runtime/lib/js_helper.dart 1128:37                                                        Object.wrapException
/_internal/js_runtime/lib/js_helper.dart 2596:3                                                         Object.assertThrow
/packages/over_react/src/component_declaration/component_type_checking.dart 175:3                       Object.getComponentTypeMeta
/packages/over_react/src/util/react_wrappers.dart 143:28                                                Object.getProps

For Fragment ReactElements (Fragment()(...)), .type is apparently JS Symbol instance, and is a case not handled by this check causing this assertion to fail.

This case should not throw.

Also, we just shouldn't try to validate the .type that originates from ReactElements; if we don't make any assumptions about what type it can be, this issue won't occur.

Changes

• Remove unnecessary assert as a workaround for false positive with Fragment, add TODOs

  isPotentiallyValidComponentType is incorrect in dart2js (see TODO on that function) and it returns false for Fragment's Symbol type.

  There's not a great way to fix that behavior until Dart 3 (see TODO), so in the meantime, we'll just remove this assert.

  The assert really isn't necessary, since only guaranteed-to-be-valid component types from ReactElements and ReactComponentFactoryProxys get passed into it, as opposed to consumer arguments, so we should be able to safely remove it. Any cases it might have caught should also be covered by test coverage on type checking code.

• Add regression test

  Note: due to current behavioral differences of isPotentiallyValidComponentType between dart2js and DDC, these regression tests only fail in dart2js with --enable-asserts, which we don't want to run our tests with.

  As opposed to adding a whole new build configuration and updating our test setup just for that one case, which we'll be able to fix once we move to Dart 3, we'll just verify this manually as part of QAing this PR.

Release Notes

Fix false positive assert in dart2js (when using --enable-asserts) when passing a Fragment into getProps(..., traverseWrappers: true)

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:

    • Verify locally that regression tests fail in dart2js with --enable-asserts, before the fix.
      Normally you'd need to update build.yaml, but you can also use --build-args, which I've done here for convenience. Just run the following commands:

      $ git checkout 698779f
      $ dart run dart_dev test --release \
          --build-args='--define=build_web_compilers|entrypoint=dart2js_args=["--enable-asserts","--no-minify"]' \
          -N 'type checking and related utilities' test/over_react_component_declaration_test.dart

      You should see failures that look like this: https://gist.github.com/greglittlefield-wf/afb337f5b9ccf15f93be5a3a2d956edc

    • Verify locally that regression tests (and other tests in suite) pass in dart2js with --enable-asserts, after the fix.
      Use the following commands:

      $ git checkout fix-fragment-type-check-assert
      $ dart run dart_dev test --release \
          --build-args='--define=build_web_compilers|entrypoint=dart2js_args=["--enable-asserts","--no-minify"]' \
          --test-args='--exclude-tags=ddc,no-ddc' \
          test/over_react_component_declaration_test.dart

  • Anything falling under manual testing criteria outlined in CONTRIBUTING.md

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

[aaronlademann-wf]

#nit has the ticket for this already been created?

[greglittlefield-wf]

Nope, I hadn't made one, wasn't sure if it was worth it or if we could just roll it in with other updates when we rip out pre-3.0.0 code

[aaronlademann-wf]

QA +1

• Verify locally that regression tests fail in dart2js with --enable-asserts, before the fix.
• Verify locally that regression tests (and other tests in suite) pass in dart2js with --enable-asserts, after the fix.

@Workiva/release-management-pp

[rmconsole-wf]

+1 from RM