Lighthouse: Unused '@ts-expect-error' directive in 6.2.0

[oddui]

Summary

I use lighthouse in a typescript project with the following ts configs

"allowJs": true,
"maxNodeModuleJsDepth": 1,

The JSDoc types in lighthouse has been working nicely before 6.2.0.

On version 6.2.0, I start to see Unused '@ts-expect-error' directive errors

node_modules/lighthouse/lighthouse-core/gather/driver.js:296:5 - error TS2578: Unused '@ts-expect-error' directive.

296     // @ts-expect-error TODO(bckenny): tsc can't type event.params correctly yet,
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/lighthouse/lighthouse-core/gather/gatherers/gatherer.js:25:5 - error TS2578: Unused '@ts-expect-error' directive.

25     // @ts-expect-error - assume that class name has been added to LH.GathererArtifacts.
       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/lighthouse/lighthouse-core/lib/dependency-graph/base-node.js:132:5 - error TS2578: Unused '@ts-expect-error' directive.

132     // @ts-expect-error - in checkJs, ts doesn't know that CPUNode and NetworkNode *are* BaseNodes.
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/lighthouse/lighthouse-core/lib/dependency-graph/base-node.js:267:5 - error TS2578: Unused '@ts-expect-error' directive.

267     // @ts-expect-error - only traverses graphs of Node, so force tsc to treat `this` as one
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/lighthouse/lighthouse-core/lib/dependency-graph/base-node.js:273:7 - error TS2578: Unused '@ts-expect-error' directive.

273       // @ts-expect-error - queue has length so it's guaranteed to have an item
          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/lighthouse/lighthouse-core/lib/dependency-graph/base-node.js:309:7 - error TS2578: Unused '@ts-expect-error' directive.

309       // @ts-expect-error - toVisit has length so it's guaranteed to have an item
          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...

It seems the errors are happening because I don't have "checkJs": true ts config set, as a result typescript raises errors for all the @ts-expect-error in js files. However setting"checkJs": true would cause typescript to raise errors all over the place for other js libraries.

I'm wondering if it's possible to change @ts-expect-error back to @ts-ignore? Or are there other ways to work around it?

Thanks

[connorjclark]

Can't you exclude node_modules from type checking?

[oddui]

Hi @connorjclark thank you for your reply. I'm not sure how to exclude node_modules while still being able to use the types defined in lighthouse.

Here is a code snippet and my tsconfig.json file. Can you show me how to do that?

import NetworkRecorder from 'lighthouse/lighthouse-core/lib/network-recorder';
import NetworkRequest from 'lighthouse/lighthouse-core/lib/network-request';
import Digraph from './Digraph';

export default class NetworkGraph extends Digraph<NetworkRequest> {
  static recordsFromLogs(devtoolsLog: LH.DevtoolsLog): NetworkRequest[] {
    return NetworkRecorder.recordsFromLogs(devtoolsLog);
  }

  constructor(requests: NetworkRequest[]) {
    super(requests);
  }
}

{
  "compilerOptions": {
    "target": "es2015",
    "lib": [
      "dom",
      "dom.iterable",
      "esnext"
    ],
    "strict": true,
    "noImplicitReturns": true,
    "noUnusedLocals": true,
    "forceConsistentCasingInFileNames": true,
    "esModuleInterop": true,
    "resolveJsonModule": true,
    "moduleResolution": "node",
    "allowJs": true,
    "maxNodeModuleJsDepth": 1,
    "jsx": "react",
    "sourceMap": true,
    "outDir": "dist"
  }
}

[brendankenny]

Hi @oddui

I'm not sure if there's a great way for us to fix this:

• We want to use the benefits of @ts-expect-error (like self-alerting when they can be removed), so I don't think we'll want to move back to @ts-ignore.
• We've also long talked about emitting type declaration files for consumers of lighthouse (#1773), but the plan there was probably not going to include internal files like network-recorder because we don't necessarily want to commit to their current interface as a public API/requiring major version bumps for changes.

Possible other options:

• write your own .d.ts declarations for those files (possibly automatically generated with --emitDeclarationOnly?). Annoying to do yourself and possibly fragile, but it should work
• file a bug on typescript to not error on unused @ts-expect-error in js files when checkJs isn't enabled. This does seem bug-like from at least your description of the situation (although I could be missing some of the pieces), but I also think there's a good chance the issue would be closed with the recommendation to always use d.ts files for dependencies (either from the dependency itself or from DefinitelyTyped). Maybe worth trying, though?

Anyone reading this have other ideas?

[patrickhulce]

Isn't this the exact purpose of typescript's skipLibCheck option? To insulate consumers from the specific options necessary to validate a library's types?

I'm not sure if it extends to JSDoc declarations or stops at just .d.ts files, but that would be my first attempt. Otherwise a bug in typescript does sound like the proper next step :)

[brendankenny]

Isn't this the exact purpose of typescript's skipLibCheck option? To insulate consumers from the specific options necessary to validate a library's types?

I think I've only ever heard of folks using that with d.ts files, not source files with declared types, but it's worth trying 🤷

[brendankenny]

Fixed in https://github.com/microsoft/TypeScript/pull/40046

Thanks @k-yle!