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-3254 Officially support Dart 3 #958

Merged
merged 25 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
4b7627b
Start running CI on Dart 3
greglittlefield-wf Oct 10, 2024
b57ea35
Fix permissions
greglittlefield-wf Oct 10, 2024
b9788c2
Fix formatting check
greglittlefield-wf Oct 10, 2024
36fc646
Fix warnings in Dart 3 (except for in generated files)
greglittlefield-wf Oct 10, 2024
78a3004
Remove test cases that produce warnings in Dart 3
greglittlefield-wf Oct 10, 2024
c1ad5da
Remove unused dependencies
greglittlefield-wf Oct 10, 2024
50abd3f
Widen build_web_compilers range to fix DDC compilation on Dart 3
greglittlefield-wf Oct 10, 2024
ff88b62
Fix Dart-2-specific import
greglittlefield-wf Oct 10, 2024
d754ed8
Exclude non-null-safe builder test cases in Dart 3
greglittlefield-wf Oct 10, 2024
105f01b
Fix test args not being passed
greglittlefield-wf Oct 10, 2024
32c5b36
Suppress analysis_options include warning in Dart 3
greglittlefield-wf Oct 10, 2024
9f53995
Fix empty string being passed as test arg
greglittlefield-wf Oct 10, 2024
5412fef
Remove Dart-2-only lint config causing warning in Dart 3
greglittlefield-wf Oct 10, 2024
922d535
Work around presets ignoring excluded tag args
greglittlefield-wf Oct 10, 2024
3a6933a
Suppress comment references info
greglittlefield-wf Oct 10, 2024
25622a5
Use Dart stable (main used by mistake)
greglittlefield-wf Oct 10, 2024
0331324
Add timeouts, helpful for when test process hangs at end of run
greglittlefield-wf Oct 10, 2024
f6719b2
Work around test hanging issue
greglittlefield-wf Oct 15, 2024
9d1cdd2
Fix Dart 3 CI trying to run unsound test file
greglittlefield-wf Oct 16, 2024
35e8c9c
Only run SBOM action once
greglittlefield-wf Oct 16, 2024
bfa632a
Dial back matrix to only run on latest Dart 3 version
greglittlefield-wf Oct 16, 2024
bcbb6ea
Fix typo
greglittlefield-wf Oct 16, 2024
375848e
Clean up temporary file
greglittlefield-wf Oct 16, 2024
d64668c
Officially support Dart 3 in SDK constraint
greglittlefield-wf Oct 16, 2024
8df1dc0
Fix error message assertion in Dart 3
greglittlefield-wf Oct 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 65 additions & 17 deletions .github/workflows/dart_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,46 @@ permissions:
pull-requests: write

jobs:
# Run as a separate job outside the Dart SDK matrix below,
# since we can only emit a single SBOM.
create-sbom-release-asset:
name: Create SBOM Release Asset
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: dart-lang/setup-dart@v1
with:
sdk: 2.19.6 # This version doesn't matter so long as it resolves.
- run: dart pub get
- name: Publish SBOM to Release Assets
uses: anchore/sbom-action@v0
with:
path: ./
format: cyclonedx-json

build:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
# Can't run on `dev` (Dart 3) until we're fully null-safe.
sdk: [ 2.19.6 ]
sdk: [ 2.19.6, stable ]
steps:
- uses: actions/checkout@v4
- uses: dart-lang/setup-dart@v1
- id: setup-dart
uses: dart-lang/setup-dart@v1
with:
sdk: ${{ matrix.sdk }}

- name: Print Dart SDK version
run: dart --version

- name: Delete Dart-2-only files when running on Dart 3
run: |
DART_VERSION="${{ steps.setup-dart.outputs.dart-version }}"
if [[ "$DART_VERSION" =~ ^3 ]]; then
./tool/delete_dart_2_only_files.sh
fi
- id: install
name: Install dependencies
run: dart pub get
Expand All @@ -44,7 +68,7 @@ jobs:
- name: Verify formatting
run: dart run dart_dev format --check
# Only run on one sdk version in case there are conflicts
if: always() && matrix.sdk != '2.19.6' && steps.install.outcome == 'success'
if: always() && matrix.sdk == '2.19.6' && steps.install.outcome == 'success'

# Analyze before generated files are created to verify that component boilerplate analysis is "clean" without the need for building
- name: Analyze example source (pre-build)
Expand Down Expand Up @@ -81,42 +105,52 @@ jobs:
- name: Run tests (VM)
# Can't use build_runner (which dart_dev uses if you depend on it) to run VM tests, since we get the error:
# Unable to spawn isolate: /…/build_runner_testRU6M77/.packages: Error: Problem in packages configuration file: Unexpected character
run: dart test -P vm
run: |
DART_VERSION="${{ steps.setup-dart.outputs.dart-version }}"
TEST_ARGS=""
if [[ "$DART_VERSION" =~ ^3 ]]; then
TEST_ARGS="--preset=no-dart-2"
fi
dart test --preset vm $TEST_ARGS
if: always() && steps.install.outcome == 'success' && steps.build.outcome == 'success'

- name: Run tests (DDC)
run: dart test --precompiled ddc_precompiled -P dartdevc
if: always() && steps.install.outcome == 'success' && steps.build.outcome == 'success'
timeout-minutes: 5

- name: Run tests (dart2js)
run: dart run dart_dev test --build-args="-r" -P dart2js
if: always() && steps.install.outcome == 'success' && steps.build.outcome == 'success'

- uses: anchore/sbom-action@v0
with:
path: ./
format: cyclonedx-json
timeout-minutes: 5

validate_analyzer:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
# Can't run on `dev` (Dart 3) until we're fully null-safe.
sdk: [ 2.19.6 ]
sdk: [ 2.19.6, stable ]
analyzer:
# We only have one version currently, but we'll leave this CI step in place
# for the next time we need to support multiple analyzer versions.
- ^5.1.0
steps:
- uses: actions/checkout@v4
- uses: dart-lang/setup-dart@v1
- id: setup-dart
uses: dart-lang/setup-dart@v1
with:
sdk: ${{ matrix.sdk }}

- name: Print Dart SDK version
run: dart --version

- name: Delete Dart-2-only files when running on Dart 3
run: |
DART_VERSION="${{ steps.setup-dart.outputs.dart-version }}"
if [[ "$DART_VERSION" =~ ^3 ]]; then
./tool/delete_dart_2_only_files.sh
fi
- name: Update analyzer constraint to ${{ matrix.analyzer }} and validate `dart pub get` can resolve
id: resolve
run: |
Expand All @@ -132,7 +166,13 @@ jobs:
run: dart run build_runner build --build-filter='**.dart' --delete-conflicting-outputs

- name: Run builder tests
run: dart test -p vm -- test/vm_tests/builder
run: |
DART_VERSION="${{ steps.setup-dart.outputs.dart-version }}"
TEST_ARGS=""
if [[ "$DART_VERSION" =~ ^3 ]]; then
TEST_ARGS="--preset=no-dart-2"
fi
dart test --preset vm $TEST_ARGS -- test/vm_tests/builder
analyzer_plugin:
runs-on: ubuntu-latest
Expand All @@ -142,17 +182,24 @@ jobs:
strategy:
fail-fast: false
matrix:
# Can't run on `stable` (Dart 3) until we're fully null-safe.
sdk: [ 2.19.6 ]
sdk: [ 2.19.6, stable ]
steps:
- uses: actions/checkout@v4
- uses: dart-lang/setup-dart@v1
- id: setup-dart
uses: dart-lang/setup-dart@v1
with:
sdk: ${{ matrix.sdk }}

- name: Print Dart SDK version
run: dart --version

- name: Delete Dart-2-only files when running on Dart 3
run: |
DART_VERSION="${{ steps.setup-dart.outputs.dart-version }}"
if [[ "$DART_VERSION" =~ ^3 ]]; then
(cd ../.. && ./tool/delete_dart_2_only_files.sh)
fi
- id: link
name: Override over_react dependency with local path
run: cd ../.. && dart pub get && dart tool/travis_link_plugin_deps.dart
Expand All @@ -177,3 +224,4 @@ jobs:
- name: Run tests
run: dart run dart_dev test
if: always() && steps.install.outcome == 'success'
timeout-minutes: 8
3 changes: 3 additions & 0 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ analyzer:
unused_import: warning
unnecessary_import: warning
unnecessary_null_comparison: warning
comment_references: info
# To work around warning "'can_be_null_after_null_aware' isn't a recognized error code" in Dart 3
included_file_warning: info

# Workaround for https://github.com/dart-lang/sdk/issues/51087
# TODO remove once we're no longer running CI on Dart 2.18
Expand Down
10 changes: 8 additions & 2 deletions dart_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,18 @@ platforms:
- chrome
- vm

# Default concurrency of 4 for unit tests, integration preset will override
concurrency: 4
# Work around process hanging after tests finish: https://github.com/dart-lang/build/issues/3765
concurrency: 1

presets:
vm:
concurrency: 4
paths:
- test/vm_tests/

no-dart-2:
exclude_tags: dart-2-only

dartdevc:
exclude_tags: no-ddc
paths:
Expand Down Expand Up @@ -48,3 +52,5 @@ tags:
# Tests that represent a case of undesirable JS interop behavior that's
# either unavoidable or exists as a tradeoff for improved behavior in other cases.
js-interop-tradeoff: {}
# Tests for behavior that's only valid in Dart 2, and does not apply to Dart 3.
dart-2-only: {}
4 changes: 0 additions & 4 deletions lib/src/component_declaration/disposable_manager_proxy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,21 +63,18 @@ mixin DisposableManagerProxy on react.Component implements DisposableManagerV7 {
_getDisposableProxy().listenToStream(stream, onData,
onError: onError, onDone: onDone, cancelOnError: cancelOnError);

@override
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These weren't actually overrides and were infos in Dart 2, but got upgraded to warnings in Dart 3.

Disposable manageAndReturnDisposable(Disposable disposable) =>
_getDisposableProxy().manageAndReturnTypedDisposable(disposable);

@override
Completer<T> manageCompleter<T>(Completer<T> completer) =>
_getDisposableProxy().manageCompleter<T>(completer);

@override
void manageDisposable(Disposable disposable) =>
_getDisposableProxy().manageDisposable(disposable);

/// DEPRECATED. Use [getManagedDisposer] instead.
@Deprecated('w_common 2.0.0')
@override
void manageDisposer(Disposer disposer) =>
_getDisposableProxy().getManagedDisposer(disposer);

Expand All @@ -87,7 +84,6 @@ mixin DisposableManagerProxy on react.Component implements DisposableManagerV7 {

/// DEPRECATED. Use [listenToStream] instead.
@Deprecated('w_common 2.0.0')
@override
void manageStreamSubscription(StreamSubscription subscription) =>
_getDisposableProxy().getManagedDisposer(subscription.cancel);

Expand Down
2 changes: 1 addition & 1 deletion lib/src/util/css_value_util.dart
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class CssValue implements Comparable<CssValue> {

/// Returns whether this value's [number] and [unit] are equal to that of [other].
@override
bool operator ==(dynamic other) {
bool operator ==(Object other) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a new built-in Dart warning; apparently == will never be passed a null value (== null bypasses operator==).

Copy link
Member

Choose a reason for hiding this comment

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

interesting

return identical(this, other) || (other is CssValue && number == other.number && unit == other.unit);
}

Expand Down
2 changes: 2 additions & 0 deletions lib/src/util/prop_conversion.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// This is only an unnecessary import in Dart 3; in Dart 2, we need it for allowInterop.
// ignore: unnecessary_import
import 'package:js/js.dart';
import 'package:js/js_util.dart';
import 'package:meta/meta.dart';
Expand Down
6 changes: 2 additions & 4 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ version: 5.4.0
description: A library for building statically-typed React UI components using Dart.
homepage: https://github.com/Workiva/over_react/
environment:
sdk: '>=2.19.0 <3.0.0'
sdk: '>=2.19.0 <4.0.0'

dependencies:
collection: ^1.15.0
Expand All @@ -30,15 +30,13 @@ dev_dependencies:
build_resolvers: ^2.0.0
build_runner: ^2.0.0
build_test: ^2.0.0
build_web_compilers: ^3.0.0
built_value_generator: ^8.0.0
build_web_compilers: '>=3.2.6 <5.0.0'
dart_dev: ^4.0.1
dependency_validator: ^3.0.0
glob: ^2.0.1
io: ^1.0.0
react_testing_library: ^3.0.1
over_react_test: ^3.0.0
pedantic: ^1.11.1
test: ^1.20.0
workiva_analysis_options: ^1.4.0
yaml: ^3.1.0
Expand Down
4 changes: 2 additions & 2 deletions test/over_react/component/lazy_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,8 @@ sharedNestedPropConversionTests(UiFactory<TestJsProps> builder, {bool isJsCompon
// DDC error message
matches(
RegExp(r"Expected a value of type 'Map[^']*', but got one of type '(Native|Legacy)JavaScriptObject'")),
// dart2js error message
matches(RegExp(r"type '(Unknown|Plain)JavaScriptObject' is not a subtype of type 'Map[^']*'")),
// Dart 2 dart2js, Dart 3 DDC error message
matches(RegExp(r"type '(Unknown|Plain|Legacy)JavaScriptObject' is not a subtype of type 'Map[^']*'")),
)),
]);
if (!isJsComponent) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,6 @@ void main() {
testValue: 'test value',
);
});
test('nullable dynamic with extraneous ? syntax', () {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These test cases tested a dynamic? type as an edge case, which now warns instead of infos when used (because the ? is unnecessary).

I tried suppressing those warnings, but dynamic? also appears in the generated part file, and there's no easy way to suppress that warning for just that file.

Since this was an edge-case that warns when the user authors it, preserving the exact type is covered by other test cases, and we originally added this case "just in case", I opted to just remove this case here and in other files.

testPropWriteAndRead<dynamic>(
readProp: (p) => p.nullableDynamicWithQuestion,
writeProp: (p, value) => p.nullableDynamicWithQuestion = value,
testValue: 'test value',
);
});
test('nullable typedef, without ? syntax', () {
testPropWriteAndRead<NullableTypedef>(
readProp: (p) => p.nullableTypedefWithoutQuestion,
Expand Down Expand Up @@ -171,9 +164,6 @@ void main() {
test('nullable dynamic', () {
expectReadPropReturnsNull((props) => props.nullableDynamic);
});
test('nullable dynamic with extraneous ? syntax', () {
expectReadPropReturnsNull((props) => props.nullableDynamicWithQuestion);
});
test('nullable typedef, without ? syntax', () {
expectReadPropReturnsNull((props) => props.nullableTypedefWithoutQuestion);
});
Expand Down Expand Up @@ -290,13 +280,6 @@ void main() {
testValue: 'test value',
);
});
test('nullable dynamic with extraneous ? syntax', () {
testStateWriteAndRead<dynamic>(
readState: (p) => p.nullableDynamicWithQuestion,
writeState: (p, value) => p.nullableDynamicWithQuestion = value,
testValue: 'test value',
);
});
test('nullable typedef, without ? syntax', () {
testStateWriteAndRead<NullableTypedef>(
readState: (p) => p.nullableTypedefWithoutQuestion,
Expand Down Expand Up @@ -365,9 +348,6 @@ void main() {
test('nullable dynamic', () {
expectReadStateReturnsNull((state) => state.nullableDynamic);
});
test('nullable dynamic with extraneous ? syntax', () {
expectReadStateReturnsNull((state) => state.nullableDynamicWithQuestion);
});
test('nullable typedef, without ? syntax', () {
expectReadStateReturnsNull((state) => state.nullableTypedefWithoutQuestion);
});
Expand Down Expand Up @@ -396,8 +376,6 @@ class _$NullSafeTestProps extends UiProps {
String? nullable;
dynamic nullableDynamic;

// ignore: unnecessary_question_mark
dynamic? nullableDynamicWithQuestion;
NullableTypedef nullableTypedefWithoutQuestion;
}

Expand All @@ -412,8 +390,6 @@ class _$NullSafeTestState extends UiState {
String? nullable;
dynamic nullableDynamic;

// ignore: unnecessary_question_mark
dynamic? nullableDynamicWithQuestion;
NullableTypedef nullableTypedefWithoutQuestion;
}

Expand Down
Loading