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-2040 Placeholder null safety migration guide #910

Merged
merged 8 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ __Once you have migrated your components to `UiComponent2`__, you're ready to st
- [__BuiltRedux to Redux__](doc/built_redux_to_redux.md): A guide to transitioning to OverReact Redux from BuiltRedux.
- [__Flux to Redux__](doc/flux_to_redux.md): A guide to how to transition from w_flux to OverReact Redux. This guide also introduces a new architecture, Influx, that can be used for incremental refactors.
- [__Dart2 Migration__](doc/dart2_migration.md): Documentation on the Dart 2 builder updates and how to transition componentry to Dart 2.
- [__Null Safety__](doc/null_safety/null_safe_migration.md): Documentation on how to migrate OverReact code to null safety.

 
 
Expand Down
84 changes: 84 additions & 0 deletions doc/null_safety/null_safe_migration.md
greglittlefield-wf marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
# Dart Null Safety Migration Guide

> [!NOTE]
> For Workiva employees, also refer to this more detailed Workiva-specific [migration guide](https://github.com/Workiva/dart_null_tools/blob/master/workiva_migration_guide.md).

Steps to migrate a repo to null safety:
greglittlefield-wf marked this conversation as resolved.
Show resolved Hide resolved

1. [View resources](#step-1-view-resources)
1. [Run the preparation codemod](#step-2-run-the-preparation-codemod-coming-soon)
1. [Run the migration tool](#step-3-run-the-migration-tool)

## Step 1: View resources

If you haven't already, familiarize yourself with the concepts of Dart null safety with the following resources:

- [Understanding null safety](https://dart.dev/null-safety/understanding-null-safety) article

<!-- TODO - add links to over_react null safety documentation once it exists -->

## Step 2: Run the preparation codemod (coming soon!)

<!-- TODO - update this section once we release the `null_safety_prep` codemod. -->

> [!WARNING]
> This codemod does not exist yet, but is coming soon!

Start out by running the `null_safety_prep` [codemod][orcm] on your repo:

```
dart pub global activate over_react_codemod
dart pub global run over_react_codemod:null_safety_prep
```

This codemod will:
- Migrate as many over_react specific use cases as possible.
- Get the repo into a state where the migration tool can be run with less manual intervention.

## Step 3: Run the migration tool

Run the null safety migrator tool:

- For Workiva employees, use the [Workiva version of the migrator tool](https://github.com/Workiva/wnnbd?tab=readme-ov-file#migrating-a-package).
- Otherwise, use [Dart's migrator tool](https://dart.dev/null-safety/migration-guide#migration-tool).

### Common migration cases

Below are some common cases that might come up while running the migrator tool on a repo using over_react.

#### Prop nullability

To determine if a prop should be nullable or not, first consider if the prop is required.

> [!WARNING]
> Making a prop required with the `late` keyword can be a breaking change if consumers are not always setting the prop.

Below is a table of the possible options for prop nullability:

| | Required (`late`) | Optional |
|----------------|----------------------|-----------------|
| Nullable (`?`) | `late String? prop;` | `String? prop;` |
| Non-nullable | `late String prop;` | n/a |

- (_most common_) All **optional** props should be made **nullable**.
- **Required** props can be nullable or non-nullable:
- **Nullable**: If the prop is required, but can be explicitly set to `null`.
- **Non-nullable**: If the prop is required and should never be set to `null`.

#### Potential Gotchas

The migrator tool [incorrectly treats the result of emulated function calls as nullable](https://github.com/dart-lang/sdk/issues/46263).

The most common way this affects over_react is in cases like this:
```dart
// The over_react code:
var content = Dom.div()();

// gets incorrectly migrated to:
ReactElement? content = Dom.div()();
```

<!-- TODO - not really sure how to explain this or if there is a solution besides manual overriding?
Is this fixed in the Workiva version of the tool that we should link to? -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@greglittlefield-wf do you have any thoughts on this one?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good question; we could add instructions for running the migrator tool with the fix in my branch/CL that's linked in that Dart issue.

It's a little involved since it involves cloning and partially setting up the Dart SDK, so how about I write that up and PR that separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be great! Thank you!


[orcm]: https://github.com/Workiva/over_react_codemod
Loading