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-1569 Prepare for null safety: fix lints and implicit casts #366

Merged
merged 31 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
e452d16
Upgrade lints to workiva_analysis_options v2.recommended
greglittlefield-wf Aug 9, 2023
1446d0b
dart fix --apply --code=unnecessary_new
greglittlefield-wf Aug 9, 2023
1e322c2
dart fix --apply --code=unnecessary_const
greglittlefield-wf Aug 9, 2023
acc03f1
dart fix --apply --code=prefer_single_quotes
greglittlefield-wf Aug 9, 2023
5d2793d
dart fix --apply --code=deprecated_colon_for_default_value
greglittlefield-wf Aug 9, 2023
d19f23d
dart fix --apply --code=annotate_overrides
greglittlefield-wf Aug 9, 2023
bcad3b1
dart fix --apply --code=prefer_final_in_for_each
greglittlefield-wf Aug 9, 2023
8d1a829
dart fix --apply --code=omit_local_variable_types,prefer_final_locals
greglittlefield-wf Aug 9, 2023
46a38d2
Fix missing inferred generics
greglittlefield-wf Aug 9, 2023
17b30cc
dart fix --apply --code=use_function_type_syntax_for_parameters,pref…
greglittlefield-wf Aug 9, 2023
b79d3eb
Fix more typedefs
greglittlefield-wf Aug 9, 2023
6490a9c
dart fix --apply --code=unnecessary_this
greglittlefield-wf Aug 9, 2023
1b5f972
dart fix --apply --code=unnecessary_brace_in_string_interps
greglittlefield-wf Aug 9, 2023
2948941
dart fix --apply --code=unnecessary_parenthesis
greglittlefield-wf Aug 9, 2023
911c914
Disable some more lints
greglittlefield-wf Aug 9, 2023
70429ac
Remove some unnecessary function parameter types, link to linter issue
greglittlefield-wf Aug 11, 2023
ae1914e
Ignore lints that could change behavior or break public API if fixed
greglittlefield-wf Aug 9, 2023
8b83f89
dart fix --apply --code=unused_import
greglittlefield-wf Aug 9, 2023
ae87897
dart fix --apply --code=unnecessary_import
greglittlefield-wf Aug 9, 2023
82c5d9e
Fix miscellaneous remaining lints (dart fix --apply)
greglittlefield-wf Aug 9, 2023
a7beb88
Ignore componentZone usage warnings
greglittlefield-wf Aug 9, 2023
1a6d534
Remove unused code
greglittlefield-wf Aug 9, 2023
9949639
Fix warnings by hand
greglittlefield-wf Aug 9, 2023
5344c86
Fix comment references and links - all credit to @aaronlademann-wf
aaronlademann-wf Aug 9, 2023
e1e3f7f
Fix implicit casts, clean up variable types and `final`
greglittlefield-wf Aug 10, 2023
35297ff
Format
greglittlefield-wf Aug 11, 2023
94d3507
Remove return types on overrides so they can be inferred
greglittlefield-wf Aug 11, 2023
cf5e31a
Remove return types on overridden getters so they can be inferred
greglittlefield-wf Aug 11, 2023
b778dcd
Format
greglittlefield-wf Aug 21, 2023
920f9b1
Disable Dart 3 CI run for now
greglittlefield-wf Aug 21, 2023
60aec37
Add comment LHS-typed function declaration
greglittlefield-wf Aug 22, 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
4 changes: 2 additions & 2 deletions .github/workflows/dart_ci.yml
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reminder to review commits one-by-one!

This should make review much easier. See PR description for more info

Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ jobs:
strategy:
fail-fast: false
matrix:
# Can't run on `beta` (Dart 3) until we're fully null-safe.
sdk: [2.18.7, stable]
# Can't run on `stable` (Dart 3) until we're fully null-safe.
sdk: [2.18.7]
steps:
- uses: actions/checkout@v2
- uses: dart-lang/setup-dart@v1
Expand Down
27 changes: 20 additions & 7 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,25 @@
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-dynamic: true
errors:
must_call_super: error
comment_references: info
# This is too noisy since it warns for all lifecycle methods.
always_declare_return_types: ignore
# We very often need to annotate parameters when they're embedded in Maps
# with dynamic values, so this lint is noisy and wrong in those cases.
# See: https://github.com/dart-lang/linter/issues/1099
avoid_types_on_closure_parameters: ignore
# This makes string concatenation less readable in some cases
prefer_interpolation_to_compose_strings: ignore
# The following are ignored to avoid merge conflicts with null safety branch
directives_ordering: ignore
prefer_typing_uninitialized_variables: ignore
linter:
rules:
- avoid_private_typedef_functions
- await_only_futures
- cancel_subscriptions
- close_sinks
- unawaited_futures
- avoid_init_to_null
prefer_if_elements_to_conditional_expressions: false
overridden_fields: false
type_annotate_public_apis: false
80 changes: 39 additions & 41 deletions example/geocodes/geocodes.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,30 @@ import 'dart:html';
import 'package:react/react.dart' as react;
import 'package:react/react_dom.dart' as react_dom;

/**
* Hello,
*
* This is the Dart portion of the tutorial for the Dart react package.
*
* We'll go through a simple app that queries the Google Maps API and shows the result to the user.
* It also stores the search history and allows the user to reload past queries.
*
* In this file you'll find the structure and the logic of the app. There is also a `geocodes.html` file which contains
* the mount point.
*
* Be sure that you understand the basic concepts of [React](http://facebook.github.io/react/) before reading this
* tutorial.
*
* Enjoy!
*/
/// Hello,
///
/// This is the Dart portion of the tutorial for the Dart react package.
///
/// We'll go through a simple app that queries the Google Maps API and shows the result to the user.
/// It also stores the search history and allows the user to reload past queries.
///
/// In this file you'll find the structure and the logic of the app. There is also a `geocodes.html` file which contains
/// the mount point.
///
/// Be sure that you understand the basic concepts of [React](https://reactjs.org/) before reading this
/// tutorial.
///
/// Enjoy!

/// Divide your app into components and conquer!
///
/// This is the first custom [Component].
/// This is the first custom `Component`.
///
/// It is just an HTML `<tr>` element displaying a single API response to the user.
class _GeocodesResultItem extends react.Component {
/// The only function you must implement in the custom component is [render].
///
/// Every [Component] has a map of properties called [Component.props]. It can be specified during creation.
/// Every `Component` has a map of properties called `props`. It can be specified during creation.
@override
render() {
return react.tr({}, [
Expand All @@ -42,13 +40,13 @@ class _GeocodesResultItem extends react.Component {
}
}

/// Now we need to tell ReactJS that our custom [Component] exists by calling [registerComponent].
/// Now we need to tell ReactJS that our custom `Component` exists by calling `registerComponent`.
///
/// As a reward, it gives us a function, that takes the properties and returns our element. You'll see it in action
/// shortly.
///
/// This is the only correct way to create a [Component]. Do not use the constructor!
var geocodesResultItem = react.registerComponent(() => new _GeocodesResultItem());
/// This is the only correct way to create a `Component`. Do not use the constructor!
var geocodesResultItem = react.registerComponent(() => _GeocodesResultItem());

/// In this component we'll build an HTML `<table>` element full of the `<tr>` elements generated by
/// [_GeocodesResultItem]
Expand Down Expand Up @@ -95,13 +93,13 @@ class _GeocodesResultList extends react.Component {
}
}

var geocodesResultList = react.registerComponent(() => new _GeocodesResultList());
var geocodesResultList = react.registerComponent(() => _GeocodesResultList());

/// In this [Component] we'll build a search form.
/// In this `Component` we'll build a search form.
///
/// This [Component] illustrates that:
/// This `Component` illustrates that:
///
/// > The functions can be [Component] parameters _(handy for callbacks)_
/// > The functions can be `Component` parameters _(handy for callbacks)_
///
/// > The DOM [Element]s can be accessed using `ref`s.
class _GeocodesForm extends react.Component {
Expand Down Expand Up @@ -158,16 +156,16 @@ class _GeocodesForm extends react.Component {
/// Handle form submission via `props.onSubmit`
onFormSubmit(react.SyntheticEvent event) {
event.preventDefault();
InputElement inputElement = react_dom.findDOMNode(searchInputInstance);
final inputElement = react_dom.findDOMNode(searchInputInstance);
// The input's value is accessed.
var address = inputElement.value;
final address = inputElement.value;
inputElement.value = '';
// Call the callback from the parent element is called.
props['submitter'](address);
}
}

var geocodesForm = react.registerComponent(() => new _GeocodesForm());
var geocodesForm = react.registerComponent(() => _GeocodesForm());

/// Renders an HTML `<li>` to be used as a child within the [_GeocodesHistoryList].
class _GeocodesHistoryItem extends react.Component {
Expand All @@ -189,7 +187,7 @@ class _GeocodesHistoryItem extends react.Component {
}
}

var geocodesHistoryItem = react.registerComponent(() => new _GeocodesHistoryItem());
var geocodesHistoryItem = react.registerComponent(() => _GeocodesHistoryItem());

/// Renders the "history list"
///
Expand All @@ -203,7 +201,7 @@ class _GeocodesHistoryList extends react.Component {
{
'key': 'list',
},
new List.from(props['data'].keys.map((key) => geocodesHistoryItem({
List.from((props['data'] as Map).keys.map((key) => geocodesHistoryItem({
'key': key,
'query': props['data'][key]['query'],
'status': props['data'][key]['status'],
Expand All @@ -214,11 +212,11 @@ class _GeocodesHistoryList extends react.Component {
}
}

var geocodesHistoryList = react.registerComponent(() => new _GeocodesHistoryList());
var geocodesHistoryList = react.registerComponent(() => _GeocodesHistoryList());

/// The root [Component] of our application.
/// The root `Component` of our application.
///
/// Introduces [state]. Both [state] and [props] are valid locations to store [Component] data.
/// Introduces [state]. Both [state] and [props] are valid locations to store `Component` data.
///
/// However, there are some key differences to note:
///
Expand All @@ -228,7 +226,7 @@ var geocodesHistoryList = react.registerComponent(() => new _GeocodesHistoryList
///
/// > When [state] is changed, the component will re-render.
///
/// It's a common practice to store the application data in the [state] of the root [Component]. It will re-render
/// It's a common practice to store the application data in the [state] of the root `Component`. It will re-render
/// every time the state is changed. However, it is not required - you can also use normal variables and re-render
/// manually if you wish.
///
Expand All @@ -247,23 +245,23 @@ class _GeocodesApp extends react.Component {
/// Sends the [addressQuery] to the API and processes the result
newQuery(String addressQuery) async {
// Once the query is being sent, it appears in the history and is given an id.
var id = addQueryToHistory(addressQuery);
final id = addQueryToHistory(addressQuery);

// Prepare the URL
addressQuery = Uri.encodeQueryComponent(addressQuery);
var path = 'https://maps.googleapis.com/maps/api/geocode/json?address=$addressQuery';
final path = 'https://maps.googleapis.com/maps/api/geocode/json?address=$addressQuery';

try {
// Send the request
var raw = await HttpRequest.getString(path);
final raw = await HttpRequest.getString(path);
// Delay the answer 2 more seconds, for test purposes
await new Future.delayed(new Duration(seconds: 2));
await Future.delayed(Duration(seconds: 2));
// Is this the answer to the last request?
if (id == last_id) {
// If yes, query was `OK` and `shown_addresses` are replaced
state['history'][id]['status'] = 'OK';

var data = json.decode(raw);
final data = json.decode(raw);

// Calling `setState` will update the state and then repaint the component.
//
Expand Down Expand Up @@ -291,7 +289,7 @@ class _GeocodesApp extends react.Component {

/// Add a new [addressQuery] to the `state.history` Map with its status set to 'pending', then return its `id`.
addQueryToHistory(String addressQuery) {
var id = ++last_id;
final id = ++last_id;

state['history'][id] = {'query': addressQuery, 'status': 'pending'};

Expand Down Expand Up @@ -324,7 +322,7 @@ class _GeocodesApp extends react.Component {
}
}

var geocodesApp = react.registerComponent(() => new _GeocodesApp());
var geocodesApp = react.registerComponent(() => _GeocodesApp());

/// And finally, a few magic commands to wire it all up!
///
Expand Down
2 changes: 1 addition & 1 deletion example/geocodes/geocodes.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* In this file you'll find the mount point of the app. There is also a `geocodes.dart` file which contains the
* structure and the application logic.
*
* Be sure that you understand the basic concepts of [React](http://facebook.github.io/react/) before reading this
* Be sure that you understand the basic concepts of [React](https://reactjs.org/) before reading this
* tutorial.
*
* Enjoy!
Expand Down
44 changes: 23 additions & 21 deletions example/js_components/js_components.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,33 +10,35 @@ import 'package:react/react_client/react_interop.dart';
import 'package:react/react_dom.dart' as react_dom;

main() {
var content = IndexComponent({});
final content = IndexComponent({});

react_dom.render(content, querySelector('#content'));
}

var IndexComponent = react.registerComponent2(() => new _IndexComponent());
var IndexComponent = react.registerComponent2(() => _IndexComponent());

class _IndexComponent extends react.Component2 {
SimpleCustomComponent simpleRef;

@override
get initialState => {
'open': false,
};

handleClose(_) {
this.setState({
setState({
'open': false,
});
}

handleClick(_) {
this.setState({
setState({
'open': true,
});
print(simpleRef.getFoo());
}

@override
render() {
return MuiThemeProvider(
{
Expand All @@ -45,7 +47,7 @@ class _IndexComponent extends react.Component2 {
SimpleCustom({
'foo': 'Foo Prop from dart... IN A JAVASCRIPT COMPONENT!',
'ref': (ref) {
simpleRef = ref;
simpleRef = ref as SimpleCustomComponent;
}
}),
CssBaseline({}),
Expand All @@ -62,21 +64,21 @@ class _IndexComponent extends react.Component2 {
DialogActions(
{},
Button({
'color': "primary",
'color': 'primary',
'onClick': handleClose,
}, 'OK'),
)),
Typography({
'variant': "h4",
'variant': 'h4',
'gutterBottom': true,
}, 'Material-UI'),
Typography({
'variant': "subtitle1",
'variant': 'subtitle1',
'gutterBottom': true,
}, 'example project'),
Button({
'variant': "contained",
'color': "secondary",
'variant': 'contained',
'color': 'secondary',
'onClick': handleClick,
}, Icon({}, 'fingerprint'), 'Super Secret Password'),
);
Expand All @@ -98,7 +100,7 @@ external ReactClass get _SimpleCustomComponent;
///
/// This converts the Dart props [Map] passed into it in the
/// the same way props are converted for DOM components.
final SimpleCustom = new ReactJsComponentFactoryProxy(_SimpleCustomComponent);
final SimpleCustom = ReactJsComponentFactoryProxy(_SimpleCustomComponent);

/// JS interop wrapper class for the component,
/// allowing us to interact with component instances
Expand Down Expand Up @@ -131,13 +133,13 @@ class MaterialUI {
external Map get theme;

// All the Material UI components converted to dart Components
final Button = new ReactJsComponentFactoryProxy(MaterialUI.Button);
final CssBaseline = new ReactJsComponentFactoryProxy(MaterialUI.CssBaseline);
final Dialog = new ReactJsComponentFactoryProxy(MaterialUI.Dialog);
final DialogActions = new ReactJsComponentFactoryProxy(MaterialUI.DialogActions);
final DialogContent = new ReactJsComponentFactoryProxy(MaterialUI.DialogContent);
final DialogContentText = new ReactJsComponentFactoryProxy(MaterialUI.DialogContentText);
final DialogTitle = new ReactJsComponentFactoryProxy(MaterialUI.DialogTitle);
final Icon = new ReactJsComponentFactoryProxy(MaterialUI.Icon);
final MuiThemeProvider = new ReactJsComponentFactoryProxy(MaterialUI.MuiThemeProvider);
final Typography = new ReactJsComponentFactoryProxy(MaterialUI.Typography);
final Button = ReactJsComponentFactoryProxy(MaterialUI.Button);
final CssBaseline = ReactJsComponentFactoryProxy(MaterialUI.CssBaseline);
final Dialog = ReactJsComponentFactoryProxy(MaterialUI.Dialog);
final DialogActions = ReactJsComponentFactoryProxy(MaterialUI.DialogActions);
final DialogContent = ReactJsComponentFactoryProxy(MaterialUI.DialogContent);
final DialogContentText = ReactJsComponentFactoryProxy(MaterialUI.DialogContentText);
final DialogTitle = ReactJsComponentFactoryProxy(MaterialUI.DialogTitle);
final Icon = ReactJsComponentFactoryProxy(MaterialUI.Icon);
final MuiThemeProvider = ReactJsComponentFactoryProxy(MaterialUI.MuiThemeProvider);
final Typography = ReactJsComponentFactoryProxy(MaterialUI.Typography);
5 changes: 3 additions & 2 deletions example/suspense/suspense.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ external ReactClass jsLazy(Promise Function() factory);
// Only intended for testing purposes, Please do not copy/paste this into repo.
// This will most likely be added to the PUBLIC api in the future,
// but needs more testing and Typing decisions to be made first.
ReactJsComponentFactoryProxy lazy(Future<ReactComponentFactoryProxy> factory()) => ReactJsComponentFactoryProxy(
ReactJsComponentFactoryProxy lazy(Future<ReactComponentFactoryProxy> Function() factory) =>
ReactJsComponentFactoryProxy(
jsLazy(
allowInterop(
() => futureToPromise(
Expand All @@ -32,7 +33,7 @@ ReactJsComponentFactoryProxy lazy(Future<ReactComponentFactoryProxy> factory())
);

main() {
var content = wrapper({});
final content = wrapper({});

react_dom.render(content, querySelector('#content'));
}
Expand Down
9 changes: 5 additions & 4 deletions example/test/call_and_nosuchmethod_test.dart
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
// ignore_for_file: deprecated_member_use_from_same_package
import "dart:html";
import 'dart:html';

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

class _CustomComponent extends react.Component {
@override
render() {
return react.div({}, props['children']);
}
}

var customComponent = react.registerComponent(() => new _CustomComponent());
var customComponent = react.registerComponent(() => _CustomComponent());

void main() {
react_dom.render(
Expand Down
Loading
Loading