Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FED-3253 Officially support Dart 3 #409

Merged
merged 11 commits into from
Oct 18, 2024
Merged

FED-3253 Officially support Dart 3 #409

merged 11 commits into from
Oct 18, 2024

Conversation

greglittlefield-wf
Copy link
Collaborator

@greglittlefield-wf greglittlefield-wf commented Oct 10, 2024

Related over_react PR: Workiva/over_react#958

Motivation

We should support Dart 3.

react-dart can currently be consumed in Dart 3, despite not supporting it in its SDK constraints, due to the way Dart 3 treats null-safe packages as compatible.

However, we don't officially support Dart 3, and our automated CI checks don't validate that it works properly in Dart 3.

Also, we still need to support Dart 2 for the time being, so ideally we'd be able to support both Dart 2 and 3 in the same react-dart version, and run CI against each Dart version.

Changes

  • Raise SDK constraints to explicitly support Dart 3
  • Update CI to run against both Dart 2 and Dart 3
    • In Dart 3 runs, delete non-null-safe browser test cases, since they don't apply in Dart 3 and won't compile
    • Work around test hanging issue in Dart 3
    • Break SBOM job out of matrix, since it can only get run once
  • dev_dependencies: widen build_web_compilers to allow versions needed by Dart 3

Caution

There's a bug in Dart >=3.3.0 <3.5.0 that breaks the behavior in DDC of certain functions passed to components, such as refs and potentially JS callback props: dart-lang/sdk#56897

We can't prevent users on those versions from pulling in react-dart, since they can already resolve to null-safe versions that have already been published.

So, consumers will just have to avoid those versions of Dart, or use dart2js during development or tests.

If needed, we could potentially follow up with a warning that detects this bug and provides a helpful error.

QA Checklist

  • CI runs in both Dart 2 and 3, and all steps pass

@aviary-wf
Copy link

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 greglittlefield-wf changed the title Run CI on Dart 3 Officially support Dart 3 Oct 16, 2024
@rmconsole3-wf rmconsole3-wf changed the title Officially support Dart 3 FED-3253 Officially support Dart 3 Oct 16, 2024
@greglittlefield-wf greglittlefield-wf marked this pull request as ready for review October 16, 2024 23:12
robbecker-wf
robbecker-wf previously approved these changes Oct 16, 2024
@robbecker-wf
Copy link
Member

QA+1 CI passes. Verified that Dart 3 CI runs actually run on Dart 3 and analyzer 6.

@robbecker-wf
Copy link
Member

can ignore semgrep ❌

Copy link
Member

@robbecker-wf robbecker-wf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+10

@greglittlefield-wf
Copy link
Collaborator Author

@Workiva/release-management-p

Copy link

@rmconsole-wf rmconsole-wf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 from RM

@rmconsole5-wk rmconsole5-wk merged commit b1e09e0 into master Oct 18, 2024
7 checks passed
@rmconsole5-wk rmconsole5-wk deleted the dart-3-ci branch October 18, 2024 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants