ESLint cache not breaking when expected

[samreid]

In #1424 @pixelzoom reported:

Reopening with top priority. grunt dev is failing:

% cd gas-properties
% grunt dev --brands=phet,phet-io
Running "dev" task
(Forwarding task to perennial)
Running "dev" task
info: getting dependencies.json for gas-properties
info: linting gas-properties
Fatal error: Perennial task failed:
Error: grunt lint-all in ../gas-properties failed with exit code 1
stdout:
Running "lint-all" task
[........................................] 100.00%


/Users/cmalley/PhET/GitHub/perennial-alias/js/common/ChipperVersion.d.ts
  0:0  error  Parsing error: ESLint was configured to run on `/Users/cmalley/PhET/GitHub/perennial-alias/js/common/ChipperVersion.d.ts` using `parserOptions.project`: /users/cmalley/phet/github/chipper/tsconfig/all/tsconfig.json
However, that TSConfig does not include this file. Either:
- Change ESLint's list of included files to not include this file
- Change that TSConfig to include this file
- Create a new TSConfig that includes this file and include it in your parserOptions.project
See the typescript-eslint docs for more info: https://typescript-eslint.io/linting/troubleshooting#i-get-errors-telling-me-eslint-was-configured-to-run--however-that-tsconfig-does-not--none-of-those-tsconfigs-include-this-file

/Users/cmalley/PhET/GitHub/perennial-alias/js/common/ReleaseBranch.d.ts
  0:0  error  Parsing error: ESLint was configured to run on `/Users/cmalley/PhET/GitHub/perennial-alias/js/common/ReleaseBranch.d.ts` using `parserOptions.project`: /users/cmalley/phet/github/chipper/tsconfig/all/tsconfig.json
However, that TSConfig does not include this file. Either:
- Change ESLint's list of included files to not include this file
- Change that TSConfig to include this file
- Create a new TSConfig that includes this file and include it in your parserOptions.project
See the typescript-eslint docs for more info: https://typescript-eslint.io/linting/troubleshooting#i-get-errors-telling-me-eslint-was-configured-to-run--however-that-tsconfig-does-not--none-of-those-tsconfigs-include-this-file

✖ 2 problems (2 errors, 0 warnings)

grunt dev --disable-eslint-cache and grunt dev --brands=phet,phet-io --lint=false were failing with the same errors.

I don't know if 143ff65 is supposed to be a fix, but it didn't work for me after pull.

I was able to deploy with grunt dev --brands=phet,phet-io --lint=false after deleting chipper/eslint/cache.

[samreid]

In following these steps, I was able to reproduce the situation @pixelzoom ran into yesterday:

1. comment out these lines in chipper/tsconfig/all/tsconfig.json
   // "../../../perennial-alias/js/common/ChipperVersion.d.ts",
   // "../../../perennial-alias/js/common/ReleaseBranch.d.ts",

2. in gas properties, grunt lint --disable-eslint-cache
   => error

3. restore (uncomment) those lines

4. in gas properties, grunt
   => error

I see that yesterday @pixelzoom was incorrectly advised to run grunt dev --disable-eslint-cache --brands=phet,phet-io. Today I learned based on https://github.com/phetsims/perennial/blob/main/js/grunt/dev.js that grunt dev does not propagate the --disable-eslint-cache flag, which explains the results we saw yesterday.

However:
5. in gas properties, grunt --disable-eslint-cache
=> no error

Therefore cache breaking is working correctly, and works in grunt lint, grunt lint-all and even grunt but, the flag just isn't being passed through on commands like 'grunt dev' or 'grunt rc'.

So the 2 opportunities for improvement I see in this issue are:

1. Why doesn't eslint know to break its own cache when its .eslintrc :: project :: tsconfig changes?
2. Should we propagate more flags to grunt dev?

Alternatively, we could somehow try to raise developer awareness about those "gotchas".

@pixelzoom and/or @zepumph please advise.

[zepumph]

Why doesn't eslint know to break its own cache when its .eslintrc :: project :: tsconfig changes?

This is a formal recommendation against using caching for typescript-eslint. Oh boy. . . https://typescript-eslint.io/troubleshooting/#can-i-use-eslints---cache-with-typescript-eslint. @samreid let's discuss further.

[zepumph]

@samreid and I found a very simple example of this with the rule: "@typescript-eslint/no-unnecessary-type-assertion"

// In Buoyancy
const x = Slider.getBlarg()!; // string | null -> string; 

// In Slider
  public static getBlarg(): string|null {
    return 'hi';
  }

The test:

• Do all linting from buoyancy
• grunt lint --esable-eslint-cache -> no problems
• grunt lint -> no problems (ensure that the cache is set, but maybe this one isn't needed)
• Remove the "| null" from getBlarg()
• grunt lint -> no problems
• Any changes in the sun repo don't break the cache on the buoyancy file we have the issue in.
• Make any change the the buoyancy file to break the cache (doesn't have to be more than a newline/comment/etc). Must be in that specific file.

In general @samreid and I don't think we should be running without caching during day-to-day development. In general, the cache works very well for us. There are some cases where it would be good to create a "no caching checkpoint". Perhaps something like before making an RC, before merging to main, etc.

We noted that we have discussed changing CTQ to test without caching, but don't think that is worth it. We did though notice that CT-main runs 'lint' with caching. This probably isn't a problem, since the cache operates on relative paths, but we want to make that change anyways, just to be sure.

We also want to tag phetsims/phet-info#222 in this issue, since we have found another overhead of ensuring that code changes don't negatively effect other developers. Branches is an industry standard in solving this problem, and we are excited to do more brainstorming about how to integrate that solution into our project.

We will close this issue after a PSA at dev meeting.

[pixelzoom]

@zepumph advised in #1440 (comment), so unassigning myself.

Back to @samreid for PSA at dev meeting.

[zepumph]

PSA complete. Closing