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.

Sign up for GitHub

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-3009 Update migration guide with required props codemod instructions #935

Merged
merged 1 commit into from
Aug 12, 2024
Merged

FED-3009 Update migration guide with required props codemod instructions #935

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 29 additions & 13 deletions doc/null_safety/null_safe_migration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,13 @@ 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.

### Codemod to run as part of a null safety migration
### Codemods to run as part of a null safety migration

Once you're ready to begin a null safety migration, run the `null_safety_migrator_companion` codemod, and commit the
Once you're ready to begin a null safety migration, run the following codemods.

#### Companion codemod

Run the `null_safety_migrator_companion` codemod and commit the
hints it adds as a separate commit before proceeding with the rest of the migration.

```bash
Expand All @@ -49,12 +53,29 @@ dart pub global run over_react_codemod:null_safety_migrator_companion --yes-to-a
This codemod will:
- Add nullability hints to props/state that are defaulted/initialized in class components.
- These hints will cause defaulted/initialized values to be migrated as "late required".
See our [prop nullability](#prop-nullability) docs for more details on whether you should keep them required following the migration.
See our [prop requiredness and nullability](#prop-requiredness-and-nullability) docs for more details on whether you should keep them required following the migration.

#### Required props codemod

Run the `null_safety_required_props` codemod and commit the hints it adds
as a separate commit before proceeding with the rest of the migration.

This is a two-step process involving two sub-commands:

1. `null_safety_required_props collect` - Collects requiredness data for all OverReact props based on usages in the specified packages and all their transitive dependencies.
1. `null_safety_required_props codemod` - Adds null safety migrator hints to OverReact props using prop requiredness data from 'collect' command.

Start with the `collect` command, following its help output for instructions:
```shell
dart pub global activate over_react_codemod
# The --help output will provide more detailed instructions.
dart pub global run over_react_codemod:null_safety_required_props collect --help
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The --help output is where I chose to document instructions for this codemod, so I figured we'd just use that as the source of truth as opposed to having to maintain a copy of the instructions here.

I'm open to feedback if people don't like this idea.

For reference, here's the latest help output for the collect command:

collect command help output 
Collects requiredness data for all OverReact props based on usages in the specified packages and all their transitive dependencies.

Usage: null_safety_required_props collect [<options>] <package_spec> [<package_spec>...]
-h, --help                         Print this usage information.
    --raw-data-output-directory    An optional directory to output raw usage data file to.
-o, --output=<path>                The file to write aggregated results to.
                                   (defaults to "prop_requiredness.json")
    --verbose                      Enable verbose output.

Run "null_safety_required_props help" to see global options.

Supported package spec formats:
- Hosted pub package with optional version (uses latest if omitted):
    - `[email protected]:over_react`
    - `[email protected]:over_react#5.2.0`
- Git URL with optional revision:
    - `[email protected]:Workiva/over_react.git`
    - `https://github.com/Workiva/over_react.git`
    - `[email protected]:Workiva/over_react.git#5.2.0`
- Local file path:
    - `/path/to/over_react`
    - `file:///path/to/over_react`

Instructions
============

1. First, identify the least-common consumer(s) of OverReact components exposed by your package.

   (If all your package's components are private, you can skip the rest of this step,
   step and just use your package).

   For example, say we're dealing with package A, which is directly consumed by
   packages B, E, and F, and so on:

       A---B---C---D
       |\     /
       | E----
       \
        F---G---H

   The least-common consumers would be C (covers both B and E) and F, so we'd run:

      null_safety_required_props collect pub@…:C pub@…:F

   Note: if F were to re-export members of A, which could potentially get used
   in G, we'd do G instead of F.

      null_safety_required_props collect pub@…:C pub@…:G

   Alternatively, we could just run on D and H from the start, but if those
   packages include more transitive dependencies, then the analysis step of the
   collection process will take a bit longer.

2. If step 1 yielded more than one package, make sure all of them can resolve to
   the latest version of your package.

   If they can't, then data may be missing for recently-added props, or could be
   incorrect if props in your package were moved to different files.

   If you're not sure, try either:
   - Cloning those packages, ensuring they resolve to the latest locally,
     and providing them as local path package specs.
   - Running the command and verifying the package versions in the command output
     line up.

3. Run the 'null_safety_required_props collect' command with the packages from step 1, using
   one of the package specifier formats listed above.

4. Use the `codemod` command within the package you want to update
   (see that command's --help for instructions):

      cd my_package
      null_safety_required_props codemod --help

...which at the end, then refers to the codemod command help output:

codemod command help output 
Adds null safety migrator hints to OverReact props using prop requiredness data from 'collect' command.

Usage: null_safety_required_props codemod [<options>]
-h, --help                               Print this usage information.
    --prop-requiredness-data             The file containing prop requiredness data, collected via the 'over_react_codemod:collect' command.
                                         (defaults to "prop_requiredness.json")
    --[no-]trust-required-annotations    Whether to migrate @requiredProp and `@nullableRequiredProp` props to late required, regardless of usage data.
                                         Note that @requiredProp has no effect on function components, so these annotations may be incorrect.
                                         (defaults to on)
    --private-requiredness-threshold     The minimum rate (0.0-1.0) a private prop must be set to be considered required.
                                         (defaults to "0.95")
    --private-max-allowed-skip-rate      The maximum allowed rate (0.0-1.0) of dynamic usages of private mixins, for which data collection was skipped.
                                         If above this, all props in a mixin will be made optional (with a TODO comment).
                                         (defaults to "0.2")
    --public-requiredness-threshold      The minimum rate (0.0-1.0) a public prop must be set to be considered required.
                                         (defaults to "1")
    --public-max-allowed-skip-rate       The maximum allowed rate (0.0-1.0) of dynamic usages of public mixins, for which data collection was skipped.
                                         If above this, all props in a mixin will be made optional (with a TODO comment).
                                         (defaults to "0.05")

Codemod options
-v, --verbose                            Outputs all logging to stdout/stderr.
    --yes-to-all                         Forces all patches accepted without prompting the user. Useful for scripts.
    --fail-on-changes                    Returns a non-zero exit code if there are changes to be made. Will not make any changes (i.e. this is a dry-run).
    --stderr-assume-tty                  Forces ansi color highlighting of stderr. Useful for debugging.

Run "null_safety_required_props help" to see global options.

Instructions
============

1. First, run the 'collect' command to collect data on usages of props declared
   in your package (see that command's --help for instructions).

    null_safety_required_props collect --help

2. Run this command within the package you want to update:

    null_safety_required_props codemod

3. Inspect the TODO comments left over from the codemod. If you want to adjust
   any thresholds or re-collect data, discard changes before re-running the codemod.

4. Commit the changes made by the codemod.

5. Proceed with using the Dart null safety migrator tool to migrate your code.

6. Review TODO comments, adjusting requiredness if desired. You can use a
   find-replace with the following regex to remove them:

       ^ *// TODO\(orcm.required_props\):.+(?:\n *//  .+)*

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought the --help was enough for me to understand what to run so I'm good with that!

```

## Step 3: Enable the analyzer plugin

Enabling the analyzer plugin will help you spot potential null-safety issues in your components at analysis time,
especially as it pertains to [prop nullability](#prop-nullability).
especially as it pertains to [prop requiredness and nullability](#prop-requiredness-and-nullability).

1. Enable the plugin in your `analysis_options.yaml` file:
```yaml
Expand Down Expand Up @@ -117,10 +138,13 @@ Run the null safety migrator tool:

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

#### Prop nullability
#### Prop requiredness and nullability

First, check out our documentation around [null safety and required props](../null_safety_and_required_props.md).

The [required props codemod](#required-props-codemod) in the steps above will add hints for the migrator tool based on usage data,
but some props will need to be manually checked and adjusted.

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

> [!WARNING]
Expand All @@ -138,14 +162,6 @@ Below is a table of the possible options for prop nullability:
- **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`.

> [!TIP]
> **Determining prop nullability manually can be extremely tedious and error-prone.**
>
> If you are a Workiva employee, and you don't want to have to go through the process shown
> in the decision tree below for every prop in every component in your library, you should wait for the
> [codemod that is being built](https://jira.atl.workiva.net/browse/FED-1885) before migrating your UI
> components since it will automate the vast majority of this process.

```mermaid
---
title: Should My Prop Be Required?
Expand Down