Skip to content

Conversation

@greglittlefield-wf
Copy link
Contributor

@greglittlefield-wf greglittlefield-wf commented Jun 27, 2025

Motivation

over_react supports analyzer >=5.13.0 <7.0.0, which doesn't include the latest 7.x releases.

We want to support the latest analyzer, to unblock upgrading packages that depend on newer analyzer versions.

Changes

  • Update analyzer constraint to allow 7.x - no consumptions of package:analyzer in over_react were affected
  • Update CI to validate analyzer 7.x
  • Raise dart_style upper bound to allow 3.x, since 2.x isn't compatible with analyzer 7.x
    • Add lib/src/builder/dart_style_compat.dart as a workaround so we can be compatible with both Dart 2 and analyzer v7 in the same version of over_react - more details there.
      • This is a bit gross, but IMO preferable to having to maintain dual-release lines of over_react. Once we drop support for Dart 2, we can clean this up, along with other code and CI infrastructure we've added in the past to support both Dart 2 and 3 in the same version.
    • Update formatter logic to pass through languageVersionComment needed in dart+style 3.x
  • Update build_test dev dependency to allow 3.x to fix test loading errors in Dart 3
    • Add tool/update_tests_for_dart_3.sh workaround since tests couldn't be written to work in both 2.x and 3.x
      • Same note as above regarding grossness and avoiding dual release lines
  • Boyscouting debuggability improvements:
    • Improve failure messages in conditionally_null_safe_builder_test.dart
    • Log commands being run in delete_dart_2_only_files.sh (set -x)

Release Notes

  • Update analyzer dependency to >=5.13.0 <8.0.0 (allow v8)
  • Update dart_style dependency to >=2.0.0 <4.0.0 (allow v3)

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:
      • CI passes
      • Verify that CI now runs against analyzer v7
    • 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

@btr-rmconsole-1 btr-rmconsole-1 bot changed the title Allow analyzer 7 FED-3977 Allow analyzer 7 Jul 1, 2025
- name: Delete Dart-2-only files when running on Dart 3
- id: install
name: Install dependencies
run: dart pub get
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved pub get one step earlier in all these, since update_tests_for_dart_3.sh reads the resolved version in pubspec.lock

final nullSafetyCommentText = 'Using nullSafety: $nullSafety.${nullSafety ? ' ' : ''} $nullSafetyReason';

// Generated part files must have matching language version comments, so copy them over if they exist.
// TODO use CompilationUnit.languageVersionToken instead of parsing this manually once we're sure we can get on analyzer version 0.39.5 or greater
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed this while I was in here

@greglittlefield-wf greglittlefield-wf marked this pull request as ready for review July 10, 2025 21:04
@@ -0,0 +1,33 @@
import 'dart:mirrors';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI this isn't the first usage of dart:mirrors in over_react's builder; it's also used by instantiateAnnotation here in transformer_utils, which is used in most of the places the builder reads fields on annotations (e.g., @Props(...), @Accessor(...)).

@kealjones-wk kealjones-wk self-assigned this Jul 16, 2025
Copy link
Contributor

@sydneyjodon-wk sydneyjodon-wk left a comment

Choose a reason for hiding this comment

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

+10 🎉

  • CI passes
  • verified that all stable / Dart 3 CI runs are running on analyzer 7 as expected

@sydneyjodon-wk
Copy link
Contributor

@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

@btr-rmconsole-4 btr-rmconsole-4 bot merged commit 6c98570 into master Jul 16, 2025
10 checks passed
@btr-rmconsole-4 btr-rmconsole-4 bot deleted the analyzer-7 branch July 16, 2025 22:32
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.

6 participants