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

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

greglittlefield-wf
Copy link
Contributor

@greglittlefield-wf greglittlefield-wf commented Aug 8, 2024

Dependent PR

Depends on codemod within Workiva/over_react_codemod#290 being merged and released.


Motivation

Now that the null safety required props codemod (Workiva/over_react_codemod#290) available, we need to update instructions within the null safety migration guide.

Changes

  • Add instructions for new codemod
  • Remove placeholder about waiting for codemod to be complete

Release Notes

Review

See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.

Please review:

QA Checklist

  • Tests were updated and provide good coverage of the changeset and other affected code
  • Manual testing was performed if needed

Merge Checklist

While we perform many automated checks before auto-merging, some manual checks are needed:

  • A Frontend Frameworks Design member has reviewed these changes
  • There are no unaddressed comments - this check can be automated if reviewers use the "Request Changes" feature
  • For release PRs - Version metadata in Rosie comment is correct

@aviary-wf
Copy link

Security Insights

No security relevant content was detected by automated scans.

Action Items

  • Review PR for security impact; comment "security review required" if needed or unsure
  • Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

```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!

@greglittlefield-wf greglittlefield-wf changed the title Update migration guide with required props codemod instructions FED-3009 Update migration guide with required props codemod instructions Aug 8, 2024
@greglittlefield-wf greglittlefield-wf marked this pull request as ready for review August 8, 2024 23:09
Copy link
Contributor

@sydneyjodon-wk sydneyjodon-wk left a comment

Choose a reason for hiding this comment

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

+10 🎉

  • links work

```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

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!

@greglittlefield-wf
Copy link
Contributor Author

@Workiva/release-management-p

Copy link

@rmconsole-wf rmconsole-wf left a comment

Choose a reason for hiding this comment

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

+1 from RM

@rmconsole4-wk rmconsole4-wk merged commit 2219309 into master Aug 12, 2024
5 checks passed
@rmconsole4-wk rmconsole4-wk deleted the required-props-codemod-docs branch August 12, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants