misc: use codemod for optional chaining

[connorjclark]

I used this tool: https://github.com/villesau/optional-chaining-codemod

First commit is just running the tool on our code, no manual changes except reverting changes to compiled test files. Subsequent commits may be done for some touchups.

Misses a few files, because of villesau/optional-chaining-codemod#48

[connorjclark]

any interest in an eslint rule too?

[brendankenny]

A quick scan of this doesn't appear to have anything I wouldn't write myself, but for all these Node version bumps I'm always more in favor of letting code update in its own time. Unless there's a big benefit or it came up in an eng sync and everyone's generally excited about it for whatever reason, it ends up in but_why.gif land for me.

It takes someone's review time and the author's time to make really sure there weren't any mistakes, and all the code being replaced honestly looks just fine. Maybe in a year or two it'll be anathema to be so liberal with our &&s (like if Promise-based code like this showed up in a PR today), but right now there's really not anything wrong with the existing working code.

But again, there's nothing here I think I wouldn't prefer and is definitely a nice cleanup in a few places. If @adamraine's excited about it that seems fine.

[adamraine]

If @adamraine's excited about it that seems fine.

I am actually excited about this. I remember the long && conditions made some of these files confusing when I first started working on Lighthouse. I think these changes will help anyone in that same situation.

[adamraine]

👍

[adamraine]

These functions seem kinda obsolete now.

[brendankenny]

If @adamraine's excited about it that seems fine.

I am actually excited about this. I remember the long && conditions made some of these files confusing when I first started working on Lighthouse. I think these changes will help anyone in that same situation.

SG.

I actually dislike the &&-existence-check pattern because it mixes control flow into types (typescript helps prevent errors with them but you still end up into these unnecessary type superpositions). ?. is basically equivalent so really should be used sparingly, but I already find myself reaching for it much more often than I do && because it's so nice and compact. Oh well :)

[brendankenny]

any interest in an eslint rule too?

thinking about this since, this seems like it might be a good idea just to avoid having to suggest it in code reviews. I can't think of a good reason for it ever to be a toss-up between the old && and optional chaining.

Is there a good rule anywhere? There's prefer-optional-chain but it's slightly stricter than what this codemod is doing (and not sure what to make of the false positive note).

[connorjclark]

Here is the output from that lint plugin on this PR.

/Users/cjamcl/src/lighthouse/build/build-report-components.js
  87:19  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/flow-report/src/common.tsx
  96:7  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-cli/test/smokehouse/frontends/lib.js
  34:26  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain
  40:5   error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/audits/byte-efficiency/uses-long-cache-ttl.js
  114:9  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/audits/script-treemap-data.js
  86:21  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/audits/seo/is-crawlable.js
  116:9  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/computed/critical-request-chains.js
  58:9  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/config/config.js
  334:20  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain
  427:7   error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/gather/gatherers/dobetterweb/tags-blocking-first-paint.js
  140:22  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/lib/asset-saver.js
  22:18  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/lib/dependency-graph/network-node.js
  50:12  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/lib/lh-error.js
  138:18  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/scripts/compare-runs.js
  310:11  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/scripts/gcp-collection/fleet-create-directories.js
  35:19  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/scripts/internal-analysis/download-issues.js
  112:25  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/test/audits/dobetterweb/no-vulnerable-libraries-test.js
  113:37  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/lighthouse-core/test/lib/tracehouse/main-thread-tasks-test.js
  215:23  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/report/renderer/performance-category-renderer.js
  373:15  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/report/renderer/report-ui-features.js
   98:28  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain
  106:7   error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain
  249:20  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain
  286:7   error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/treemap/test/treemap-test-pptr.js
  51:7   error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain
  98:13  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

/Users/cjamcl/src/lighthouse/viewer/test/viewer-test-pptr.js
   84:7   error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain
  161:30  error  Prefer using an optional chain expression instead, as it's more concise and easier to read  @typescript-eslint/prefer-optional-chain

I only checked a few, but they were all real, just for some reason missed by the codemod. We can followup in a new PR.