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-1728 Migrate to null safety #373

Merged
merged 54 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
3dd457e
Remove deprecated forwardRef
greglittlefield-wf Aug 16, 2023
cd2eda7
Remove workaround for dart-lang/sdk#27647, fixed in Dart 2
greglittlefield-wf Aug 16, 2023
ae23239
Migrate to null safety
greglittlefield-wf Aug 16, 2023
3109175
Use Never since Null is no longer the bottom type
greglittlefield-wf Aug 16, 2023
9b2379e
Fix lints
greglittlefield-wf Aug 16, 2023
f1101b7
Fix instances of `dynamic?`
greglittlefield-wf Aug 16, 2023
1c88f0f
Fix more implicit casts in examples
greglittlefield-wf Aug 16, 2023
d7cd354
Update mocks to work in null safety by using mockito's builder
greglittlefield-wf Aug 16, 2023
4176a9e
Address fixme around createReactDartComponentClass(2) nullability
greglittlefield-wf Aug 16, 2023
5adad69
Address fixme around handling a null SyntheticEvent.type
greglittlefield-wf Aug 16, 2023
af89ed3
Address fixme around accessing uninitialized late props/state fields …
greglittlefield-wf Aug 16, 2023
961aa4a
Support non-nullable refs, remove unsafe Ref constructors
greglittlefield-wf Aug 17, 2023
6024c37
Fix useRef deprecation warnings, add useRef2 tests
greglittlefield-wf Aug 18, 2023
4aed55f
Remove FIXME for Simulated event nullability (more info below)
greglittlefield-wf Aug 18, 2023
71f519a
Improve typing and safety of initComponentInternal
greglittlefield-wf Aug 22, 2023
4fa6b9d
Improve comment
greglittlefield-wf Aug 22, 2023
e7f2e9e
Make ReactDartComponentInternal.props non-nullable
greglittlefield-wf Aug 22, 2023
d807613
Eliminate ! operator from state initialization
greglittlefield-wf Aug 22, 2023
90d911e
Fix bad non-null assertion
greglittlefield-wf Aug 29, 2023
0dc260e
Improve useRef nullability by deprecating its argument instead of
greglittlefield-wf Aug 30, 2023
1a06d48
Remove duplicate PropValidator typedef, align typing
greglittlefield-wf Aug 31, 2023
f2ad196
Fix deprecated initial value in useRef not being passed along
greglittlefield-wf Aug 31, 2023
0c561dc
Pull in RefTestCase changes from over_react
greglittlefield-wf Sep 11, 2023
61cc34d
assert that callback ref arguments aren't non-nullable, add tests
greglittlefield-wf Sep 12, 2023
11ca3a7
Make chainRefs AssertionError tests more specific
greglittlefield-wf Sep 12, 2023
abe5735
Remove unused import
greglittlefield-wf Sep 12, 2023
37a47f6
Remove `implements Function` which has had no effect since Dart 2.0
greglittlefield-wf Sep 12, 2023
a6f96f8
Remove unnecessary ReactElement casts
greglittlefield-wf Sep 12, 2023
069f478
Fix most remaining implicit casts
greglittlefield-wf Sep 12, 2023
b9881a1
Add types to top-level DOM factories, clean up casts
greglittlefield-wf Sep 12, 2023
8799ed2
Add tests for how JsBackedMap handles null and other key types
greglittlefield-wf Sep 12, 2023
0d1ab08
JsBackedMap: fix errors when using null keys, fix implicit casts
greglittlefield-wf Sep 12, 2023
d708dfd
Fix find/replace typo
greglittlefield-wf Sep 12, 2023
019cabc
Add test case for `null` key within Map passed to jsifyAndAllowInterop
greglittlefield-wf Sep 12, 2023
8122645
Disable implicit casts in analysis options
greglittlefield-wf Sep 13, 2023
50e9a5e
Adjust Component2Bridge.forComponent nullability
greglittlefield-wf Sep 13, 2023
2c380ce
Tweak createContext for better type promotion
greglittlefield-wf Sep 13, 2023
e4d6570
Clean up FIXME
greglittlefield-wf Sep 13, 2023
d75e962
Format
greglittlefield-wf Oct 2, 2023
c23fffc
Merge branch 'prepare-for-7_0_0' into null-safety-manual-2
greglittlefield-wf Oct 2, 2023
e86096b
Remove deprecated StateHook and ReducerHook constructors
greglittlefield-wf Oct 2, 2023
c26f942
Fix and expand on non-nullable callback ref assertion message
greglittlefield-wf Oct 2, 2023
40a2e19
Fix logic change during migration
greglittlefield-wf Oct 2, 2023
a385e95
Use dart_dev to format so that we can exclude generated mockito files
greglittlefield-wf Oct 2, 2023
95628fc
Fix conflicting outputs in CI, also fix tests not running in dart2js
greglittlefield-wf Oct 3, 2023
086f8cb
Add missing glob dev dependency
greglittlefield-wf Oct 3, 2023
a284dfc
Seal ReducerHook, StateHook, and Ref
greglittlefield-wf Oct 4, 2023
7ca0cb8
Clean up cast
greglittlefield-wf Oct 4, 2023
dbcf43a
Fix bad generic typing on Object.keys
greglittlefield-wf Oct 10, 2023
ee90271
Address CR feedback
greglittlefield-wf Oct 11, 2023
b277b9a
Add more thorough explanation of impact of `late` member changes
greglittlefield-wf Oct 11, 2023
7ab1084
Add typedef and comments to explain StateHook _setValue typing
greglittlefield-wf Oct 12, 2023
0616cd0
Align nullability of context args in
greglittlefield-wf Oct 12, 2023
cb5cfd6
Address CR feedback
greglittlefield-wf Oct 12, 2023
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
14 changes: 11 additions & 3 deletions .github/workflows/dart_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
if: always() && steps.install.outcome == 'success'

- name: Verify formatting
run: dart format --output=none --line-length=120 --set-exit-if-changed .
run: dart run dart_dev format --check
if: ${{ matrix.sdk == '2.18.7' }}

- name: Analyze project source
Expand All @@ -44,12 +44,20 @@ jobs:

- name: Run tests (DDC)
run: |
if [ ${{ matrix.sdk }} = '2.13.4' ]; then dart run build_runner test -- --preset dartdevc-legacy; else dart run build_runner test -- --preset dartdevc; fi
if [ ${{ matrix.sdk }} = '2.13.4' ]; then
dart run build_runner test --delete-conflicting-outputs -- --preset dartdevc-legacy
else
dart run build_runner test --delete-conflicting-outputs -- --preset dartdevc
fi
if: always() && steps.install.outcome == 'success'
timeout-minutes: 5

- name: Run tests (dart2js)
run: |
if [ ${{ matrix.sdk }} = '2.13.4' ]; then dart run build_runner test -- --preset dart2js-legacy; else dart run build_runner test -- --preset dart2js; fi
if [ ${{ matrix.sdk }} = '2.13.4' ]; then
dart run build_runner test --delete-conflicting-outputs --release -- --preset dart2js-legacy
else
dart run build_runner test --delete-conflicting-outputs --release -- --preset dart2js
fi
if: always() && steps.install.outcome == 'success'
timeout-minutes: 5
38 changes: 34 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@
- markChildrenValidated

#### Other API breakages
- ReducerHook and StateHook have no public constructors and can no longer be extended
- Ref.fromJs is now a factory constructor, meaning the Ref class can no longer be extended
- `ReducerHook`, `StateHook`, and `Ref` are now `@sealed` and may not be inherited from
- ReactComponentFactoryProxy.call and .build return type changed from dynamic to ReactElement
- This matches the type returned from `build` for all subclasses, which is what’s returned by call, and reflects the type returned at runtime
- Has potential to cause some static analysis issues, but for the most part should not affect anything since ReactElement is typically treated as an opaque type
Expand All @@ -37,8 +36,39 @@
- ReactComponentFactoryProxy no longer `implements Function`
- This should not be a breakage, since as of Dart 2.0 inheriting from Function has had no effect

#### Potential behavior breakages
- Component and Component2 members `props`/`state`/`jsThis` are late, will now throw instead of being null if accessed before initialized (e.g., in a constructor, final class field, or static lifecycle method).
#### Behavior breakages unlikely to cause issues
- Component and Component2 members `props`/`state`/`jsThis` are now [late](https://dart.dev/language/variables#late-variables), and will now throw instead of being null if accessed before initialized.

It should be very uncommon for components to be affected by this change, and any affected components are likely doing something wrong to begin with.

These fields are only uninitialized:
- for mounting component instances:
- in component class constructors (which we don't encourage)
- in component class field initializers (except for lazy `late` ones)
- in "static" lifecycle methods like `getDerivedStateFromProps` and `defaultProps`

Examples of code affected:
```dart
class FooComponent extends Component2 {
// `props` would have always been null when this is initialized, but in 7.0.0 accessing it throws.
final something = (props ?? {})['something'];

// We strongly discourage declaring Dart constructors in component classes;
// for initialization logic, use componentDidMount instead.
FooComponent() {
// `props` would have always been null here, but in 7.0.0 accessing it throws.
print(props);
}

@override
getDerivedStateFromProps(nextProps, prevState) {
// `props` would have always been null here, but in 7.0.0 accessing it throws.
print(props);
return {};
}
}
```


## [6.2.1](https://github.com/Workiva/react-dart/compare/6.2.0...6.2.1)
- [#366] Fix lints and eliminate most implicit casts
Expand Down
3 changes: 1 addition & 2 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ include: package:workiva_analysis_options/v2.recommended.yaml

analyzer:
strong-mode:
# TODO change to false as part of the null safety major, which avoids us having to add a lot more casts
implicit-casts: true
implicit-casts: false
errors:
must_call_super: error
comment_references: info
Expand Down
7 changes: 3 additions & 4 deletions build.yaml
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
targets:
$default:
builders:
# mockito's builder is expensive and is not needed until this package is
# migrated to null-safety. At that point, it should be scoped only to
# relevant files.
mockito:mockBuilder:
enabled: false
# Scope only to files declaring mocks, for performance.
generate_for:
- test/mockito.dart
build_web_compilers|entrypoint:
# These are globs for the entrypoints you want to compile.
generate_for:
Expand Down
4 changes: 2 additions & 2 deletions example/js_components/js_components.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ main() {
var IndexComponent = react.registerComponent2(() => _IndexComponent());

class _IndexComponent extends react.Component2 {
SimpleCustomComponent simpleRef;
SimpleCustomComponent? simpleRef;

@override
get initialState => {
Expand All @@ -35,7 +35,7 @@ class _IndexComponent extends react.Component2 {
setState({
'open': true,
});
print(simpleRef.getFoo());
print(simpleRef!.getFoo());
}

@override
Expand Down
14 changes: 7 additions & 7 deletions example/test/function_component_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ UseRefTestComponent(Map props) {
return react.Fragment({}, [
react.p({'key': 'urtKey1'}, ['Current Input: ${inputValue.value}, Previous Input: ${prevInputValueRef.current}']),
react.input({'key': 'urtKey2', 'ref': inputRef}),
react.button({'key': 'urtKey3', 'onClick': (_) => inputValue.set(inputRef.current.value)}, ['Update']),
react.button({'key': 'urtKey3', 'onClick': (_) => inputValue.set(inputRef.current!.value!)}, ['Update']),
]);
}

Expand Down Expand Up @@ -338,13 +338,13 @@ class FancyInputApi {
}

final FancyInput = react.forwardRef2((props, ref) {
final inputRef = useRef();
final inputRef = useRef<InputElement>();

useImperativeHandle(
ref,
() {
print('FancyInput: useImperativeHandle re-assigns ref.current');
return FancyInputApi(() => inputRef.current.focus());
return FancyInputApi(() => inputRef.current!.focus());
},

/// Because the return value of createHandle never changes, it is not necessary for ref.current
Expand Down Expand Up @@ -377,14 +377,14 @@ UseImperativeHandleTestComponent(Map props) {
if (!RegExp(r'^[a-zA-Z]+$').hasMatch(city.value)) {
message.set('Invalid form!');
error.set('city');
cityRef.current.focus();
cityRef.current!.focus();
return;
}

if (!RegExp(r'^[a-zA-Z]+$').hasMatch(state.value)) {
message.set('Invalid form!');
error.set('state');
stateRef.current.focus();
stateRef.current!.focus();
return;
}

Expand Down Expand Up @@ -453,13 +453,13 @@ UseImperativeHandleTestComponent2(Map props) {
}, []),
react.button({
'key': 'button1',
'onClick': (_) => fancyCounterRef.current['increment'](),
'onClick': (_) => fancyCounterRef.current!['increment'](),
}, [
'Increment by ${diff.value}'
]),
react.button({
'key': 'button2',
'onClick': (_) => fancyCounterRef.current['decrement'](),
'onClick': (_) => fancyCounterRef.current!['decrement'](),
}, [
'Decrement by ${diff.value}'
]),
Expand Down
28 changes: 14 additions & 14 deletions example/test/react_test_components.dart
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class _CheckBoxComponent extends react.Component {
var checkBoxComponent = react.registerComponent(() => _CheckBoxComponent());

class _ClockComponent extends react.Component {
Timer timer;
late Timer timer;

@override
getInitialState() => {'secondsElapsed': 0};
Expand Down Expand Up @@ -169,7 +169,7 @@ class _ListComponent extends react.Component {
@override
render() {
final items = [];
for (final item in state['items']) {
for (final item in state['items'] as List) {
items.add(react.li({'key': item}, '$item'));
}

Expand Down Expand Up @@ -297,7 +297,7 @@ int calculateChangedBits(currentValue, nextValue) {
var TestNewContext = react.createContext<Map>({'renderCount': 0}, calculateChangedBits);

class _NewContextProviderComponent extends react.Component2 {
_NewContextRefComponent componentRef;
_NewContextRefComponent? componentRef;

@override
get initialState => {'renderCount': 0, 'complexMap': false};
Expand Down Expand Up @@ -448,7 +448,7 @@ class _Component2TestComponent extends react.Component2 with react.TypedSnapshot
}

@override
componentDidUpdate(prevProps, prevState, [String snapshot]) {
componentDidUpdate(prevProps, prevState, [String? snapshot]) {
if (snapshot != null) {
print('Updated DOM and $snapshot');
return;
Expand All @@ -473,7 +473,7 @@ class _Component2TestComponent extends react.Component2 with react.TypedSnapshot
// Used to generate unique keys even when the list contains duplicate items
final itemCounts = <dynamic, int>{};
final items = [];
for (final item in state['items']) {
for (final item in state['items'] as List) {
final count = itemCounts[item] = (itemCounts[item] ?? 0) + 1;
items.add(react.li({'key': 'c2-$item-$count'}, '$item'));
}
Expand Down Expand Up @@ -519,20 +519,20 @@ class _ErrorComponent extends react.Component2 {
var ErrorComponent = react.registerComponent(() => _ErrorComponent());

class _CustomException implements Exception {
int code;
String message;
String randomMessage;
final int code;
final String message;
final String randomMessage;

_CustomException(this.message, this.code) {
_CustomException(this.message, this.code) : randomMessage = _getRandomMessage(code);

static String _getRandomMessage(code) {
switch (code) {
case 1:
randomMessage = 'The code is a 1';
break;
return 'The code is a 1';
case 2:
randomMessage = 'The Code is a 2';
break;
return 'The Code is a 2';
default:
randomMessage = 'Default Error Code';
return 'Default Error Code';
}
}
}
Expand Down
17 changes: 8 additions & 9 deletions example/test/ref_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import 'dart:html';

import 'package:react/react.dart' as react;
import 'package:react/react_dom.dart' as react_dom;
import 'package:react/react_client.dart';

var ChildComponent = react.registerComponent(() => _ChildComponent());

Expand Down Expand Up @@ -85,36 +84,36 @@ class _ParentComponent extends react.Component {
}

// Callback refs
InputElement _inputCallbackRef;
_ChildComponent _childCallbackRef;
InputElement? _inputCallbackRef;
_ChildComponent? _childCallbackRef;
showInputCallbackRefValue(_) {
final input = react_dom.findDOMNode(_inputCallbackRef) as InputElement;
print(input.value);
}

showChildCallbackRefValue(_) {
print(_childCallbackRef.somevalue);
print(_childCallbackRef!.somevalue);
}

incrementChildCallbackRefValue(_) {
_childCallbackRef.incrementValue();
_childCallbackRef!.incrementValue();
}

// Create refs
final Ref<InputElement> _inputCreateRef = react.createRef();
final Ref<_ChildComponent> _childCreateRef = react.createRef();
final _inputCreateRef = react.createRef<InputElement>();
final _childCreateRef = react.createRef<_ChildComponent>();

showInputCreateRefValue(_) {
final input = react_dom.findDOMNode(_inputCreateRef.current) as InputElement;
print(input.value);
}

showChildCreateRefValue(_) {
print(_childCreateRef.current.somevalue);
print(_childCreateRef.current!.somevalue);
}

incrementChildCreateRefValue(_) {
_childCreateRef.current.incrementValue();
_childCreateRef.current!.incrementValue();
}

@override
Expand Down
2 changes: 1 addition & 1 deletion example/test/speed_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class _Hello extends react.Component {
@override
render() {
timeprint('rendering start');
final data = props['data'];
final data = props['data'] as List;
final children = [];
for (final elem in data) {
children.add(react.div({
Expand Down
4 changes: 2 additions & 2 deletions example/test/unmount_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ void main() {
print('What');
final mountedNode = querySelector('#content');

querySelector('#mount').onClick.listen((_) => react_dom.render(simpleComponent({}), mountedNode));
querySelector('#mount')!.onClick.listen((_) => react_dom.render(simpleComponent({}), mountedNode));
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved

querySelector('#unmount').onClick.listen((_) => react_dom.unmountComponentAtNode(mountedNode));
querySelector('#unmount')!.onClick.listen((_) => react_dom.unmountComponentAtNode(mountedNode));
}
Loading
Loading