accessibility-insights-web icon indicating copy to clipboard operation
accessibility-insights-web copied to clipboard

Codebase should use strict null checks

Open dbjorge opened this issue 5 years ago • 0 comments
trafficstars

How to help

Migrating to strict null checks is in progress, and we have some scripts (heavily based on guidance and scripts from the VS Code team) in place to help. (Note: all of these assume you have already run yarn install and yarn build once first):

$ # Verify that there are no null-safety issues in the tsconfig.strictNullChecks.json files
$ yarn null:check

$ # Start a "watch" window to continuously verify as you make changes/fixes
$ yarn null:check --watch

$ # Print a markdown-formatted checklist of files that are good candidates to try to make null-safe (ie, files with no transitive dependencies on any other non-null-safe files)
$ yarn null:find

$ # Automatically add any files null:find would output to tsconfig.strictNullChecks.json, if-and-only-if they can be added without introducing new null safety errors
$ yarn null:autoadd

The recommended workflow for contributing a null safety improvment is:

  1. On the side, start a yarn null:check --watch process
  2. Run yarn null:find to see a list of candidate files to work on
  3. Add a candidate file to the files list in tsconfig.strictNullChecks.json
  4. Fix any issues that pop up in the watch process
  5. Run yarn null:autoadd to pick up any additional files that might have been incidentally fixed as a result of your changes
  6. Optionally, repeat steps 2-5 until you have a reasonably scoped PR (try not to make them too big!)
  7. Send a PR referencing this issue
  8. Update the widget on our team standup dashboard with progress!

Background

Per 6/12/20, there was broad (but not unanimous) agreement among the engineering team that we would like to enable typescript's strictNullChecks: true config in our codebase; most participants felt that the errors that could be prevented with this particular check outweighed the engineering cost of retrofitting the code to allow it.

To enable this, we'll be following the same strategy that VS Code did; they outline the migration strategy they used in great detail in their blog post Strict null checking Visual Studio Code. To summarize it as it applies to our codebase:

  1. PR #2835 adds a new tsconfig.strictNullChecks.json that performs null checking on the set of already-strictNullChecks-clean files
  2. PR #2835 sets up stay-clean verification in yarn fastpass and PR/CI builds
  3. PR #2957 adds the migration tools to the repo
  4. We've added a widget to our team standup dashboard to track progress (for now, it needs to be manually updated as progress is made)

Potential action items upon completion:

  1. Migrate existing schema generation in LoadAssessmentDataValidator to use JSON Type Definition which should lessen the maintenance burden for the schema in the future.

dbjorge avatar Jun 12 '20 18:06 dbjorge