Run lint via npx eslint

[samreid]

From #1415 and related to #1425, @zepumph and I are interested in running lint via npx eslint. This is described in https://eslint.org/docs/latest/use/command-line-interface like so:

Most users use npx to run ESLint on the command line like this:
npx eslint [options] [file|dir|glob]*

We experimented with running this command in chipper/

time npx eslint --rulesdir ../chipper/eslint/rules/ --resolve-plugins-relative-to ../chipper --no-error-on-unmatched-pattern --ignore-path ../chipper/eslint/.eslintignore --cache --quiet ../**/js/**

It seems to be working very well, and it stays under the memory limit (with our current tsconfig/all). An uncached version takes around 4m on my machine. A cached version with no changes takes around 10 seconds

Some comments:

• --quiet is necessary to suppress warnings about files getting ignored. When we upgrade to the new "flat" configuration file, we can use --no-warn-ignore instead. But this means for now we cannot get warnings from other rules since it must be suppressed to hide the 100+ ignore warnings.
• This glob pattern lints every /js/, so if you have other repos checked out as siblings to the phet libraries, they may get picked up as well. This could be worked around by creating a smarter glob pattern that consults with active-repos. Note that const EXCLUDE_REPOS = [ 'binder', 'fenster', 'decaf', 'scenery-lab-demo' ]; are being linted by this npx process even though they don't need to be. If we expand out patterns from the perennial/data lists, we could ignore these again.
• This doesn't know or use our custom CacheLayer. We could continue to respect CacheLayer if we tap into the npx from a grunt script.
• Currently, the cache directory is set to chipper/.eslintcache. I think we should either change that or .gitignore it.

Some questions:

• How can we change our grunt lint or lint-all or lint-everything to use this entry point?
• Will adding more entry points to tsconfig/all exceed the memory limit?
• Will using repo-specific tsconfig entry points speed up the 4 minute timing?
• How many of the auxiliary functions in lint.js do we need to retain? Like chip-away, disableWithComment, progress bar?
  • According to https://stackoverflow.com/questions/70822717/how-to-print-eslint-progress-as-it-runs-but-not-everything-at-once-when-all-fil you can prepend DEBUG=eslint:cli-engine to the command to see the progress as it goes. I tested this and it may be an acceptable replacement.
• Will we want to speed up with parallelizing?

[samreid]

DEBUG=eslint:cli-engine also tells you how much time each file took:

  eslint:cli-engine Lint /Users/samreid/phet/root/equality-explorer/js/common/model/BalanceScale.ts +10ms
  eslint:cli-engine Lint /Users/samreid/phet/root/equality-explorer/js/common/model/ConstantTerm.ts +38ms
  eslint:cli-engine Lint /Users/samreid/phet/root/equality-explorer/js/common/model/ConstantTermCreator.ts +20ms
  eslint:cli-engine Lint /Users/samreid/phet/root/equality-explorer/js/common/model/EqualityExplorerModel.ts +14ms
  eslint:cli-engine Lint /Users/samreid/phet/root/equality-explorer/js/common/model/EqualityExplorerMovable.ts +19ms
  eslint:cli-engine Lint /Users/samreid/phet/root/equality-explorer/js/common/model/EqualityExplorerScene.ts +15ms

Here's one that took a bit longer:

eslint:cli-engine Lint /Users/samreid/phet/root/scenery/js/nodes/NodeTests.ts +891ms

[samreid]

Applying the specific tsconfigs from #1415 (comment) then running via DEBUG=eslint:cli-engine time npx eslint --rulesdir ../chipper/eslint/rules/ --resolve-plugins-relative-to ../chipper --no-error-on-unmatched-pattern --ignore-path ../chipper/eslint/.eslintignore --quiet ../**/js/** crashes during buoyancy. Therefore we can infer the specific tsconfig pattern consumes a lot more memory.

[samreid]

Will adding more entry points to tsconfig/all exceed the memory limit?

I added all active-sims to all/tsconfig and ran

DEBUG=eslint:cli-engine time npx eslint --rulesdir ../chipper/eslint/rules/ --resolve-plugins-relative-to ../chipper --no-error-on-unmatched-pattern --ignore-path ../chipper/eslint/.eslintignore --quiet ../**/js/**

It got up past 4GB and slowed near the end, but completed successfully without a memory error after 3.7 minutes.

[samreid]

I added npxLint. Here is a patch that uses it and adjusts the ignore patterns.

Subject: [PATCH] Add npxLint, see https://github.com/phetsims/chipper/issues/1429
---
Index: chipper/js/grunt/Gruntfile.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/grunt/Gruntfile.js b/chipper/js/grunt/Gruntfile.js
--- a/chipper/js/grunt/Gruntfile.js	(revision 6f7c6672bdf09c5b7237ed7d73874d6ae91f3547)
+++ b/chipper/js/grunt/Gruntfile.js	(date 1711131001041)
@@ -349,7 +349,7 @@
 --disable-with-comment: add an es-lint disable with comment to lint errors
 --repos: comma separated list of repos to lint in addition to the repo from running`,
     wrapTask( async () => {
-      const lint = require( './lint' );
+      const npxLint = require( './npxLint' );
 
       // --disable-eslint-cache disables the cache, useful for developing rules
       const cache = !grunt.option( 'disable-eslint-cache' );
@@ -359,7 +359,7 @@
 
       const extraRepos = grunt.option( 'repos' ) ? grunt.option( 'repos' ).split( ',' ) : [];
 
-      const lintReturnValue = await lint( [ repo, ...extraRepos ], {
+      const lintReturnValue = await npxLint( [ repo, ...extraRepos ], {
         cache: cache,
         fix: fix,
         chipAway: chipAway,
@@ -372,7 +372,7 @@
     } ) );
 
   grunt.registerTask( 'lint-all', 'lint all js files that are required to build this repository (for the specified brands)', wrapTask( async () => {
-    const lint = require( './lint' );
+    const npxLint = require( './npxLint' );
 
     // --disable-eslint-cache disables the cache, useful for developing rules
     const cache = !grunt.option( 'disable-eslint-cache' );
@@ -385,7 +385,7 @@
 
     const brands = getBrands( grunt, repo, buildLocal );
 
-    const lintReturnValue = await lint( getPhetLibs( repo, brands ), {
+    const lintReturnValue = await npxLint( getPhetLibs( repo, brands ), {
       cache: cache,
       fix: fix,
       chipAway: chipAway,
Index: perennial/js/grunt/Gruntfile.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/perennial/js/grunt/Gruntfile.js b/perennial/js/grunt/Gruntfile.js
--- a/perennial/js/grunt/Gruntfile.js	(revision 610c97d5a0204e9e822e83a02e6c4794e32c982e)
+++ b/perennial/js/grunt/Gruntfile.js	(date 1711131022310)
@@ -537,8 +537,9 @@
 
     // Don't always require this, as we may have an older chipper checked out.  Also make sure it is the promise-based lint.
     const lint = require( '../../../chipper/js/grunt/lint' );
-    if ( lint.chipperAPIVersion === 'promisesPerRepo1' ) {
-      const lintReturnValue = await lint( activeRepos, {
+    const npxLint = require( '../../../chipper/js/grunt/npxLint' );
+    if ( lint.chipperAPIVersion === 'promisesPerRepo1' && npxLint ) {
+      const lintReturnValue = await npxLint( activeRepos, {
         cache: cache,
         fix: fix,
         chipAway: chipAway,
Index: chipper/js/scripts/hook-pre-commit-task.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/scripts/hook-pre-commit-task.js b/chipper/js/scripts/hook-pre-commit-task.js
--- a/chipper/js/scripts/hook-pre-commit-task.js	(revision 6f7c6672bdf09c5b7237ed7d73874d6ae91f3547)
+++ b/chipper/js/scripts/hook-pre-commit-task.js	(date 1711131001022)
@@ -16,7 +16,7 @@
 const generatePhetioMacroAPI = require( '../phet-io/generatePhetioMacroAPI' );
 const CacheLayer = require( '../../../chipper/js/common/CacheLayer' );
 const phetioCompareAPISets = require( '../phet-io/phetioCompareAPISets' );
-const lint = require( '../../../chipper/js/grunt/lint' );
+const npxLint = require( '../../../chipper/js/grunt/npxLint' );
 const reportMedia = require( '../../../chipper/js/grunt/reportMedia' );
 const puppeteerQUnit = require( '../../../aqua/js/local/puppeteerQUnit' );
 const Transpiler = require( '../../../chipper/js/common/Transpiler' );
@@ -43,7 +43,7 @@
 
     // Run lint tests if they exist in the checked-out SHAs.
     // lint() automatically filters out non-lintable repos
-    const lintReturnValue = await lint( [ repo ] );
+    const lintReturnValue = await npxLint( [ repo ] );
     outputToConsole && console.log( `Linting passed with results.length: ${lintReturnValue.results.length}` );
     process.exit( lintReturnValue.ok ? 0 : 1 );
   }
Index: perennial-alias/js/grunt/Gruntfile.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/perennial-alias/js/grunt/Gruntfile.js b/perennial-alias/js/grunt/Gruntfile.js
--- a/perennial-alias/js/grunt/Gruntfile.js	(revision 610c97d5a0204e9e822e83a02e6c4794e32c982e)
+++ b/perennial-alias/js/grunt/Gruntfile.js	(date 1711131022316)
@@ -537,8 +537,9 @@
 
     // Don't always require this, as we may have an older chipper checked out.  Also make sure it is the promise-based lint.
     const lint = require( '../../../chipper/js/grunt/lint' );
-    if ( lint.chipperAPIVersion === 'promisesPerRepo1' ) {
-      const lintReturnValue = await lint( activeRepos, {
+    const npxLint = require( '../../../chipper/js/grunt/npxLint' );
+    if ( lint.chipperAPIVersion === 'promisesPerRepo1' && npxLint ) {
+      const lintReturnValue = await npxLint( activeRepos, {
         cache: cache,
         fix: fix,
         chipAway: chipAway,
Index: chipper/eslint/.eslintignore
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/eslint/.eslintignore b/chipper/eslint/.eslintignore
--- a/chipper/eslint/.eslintignore	(revision 6f7c6672bdf09c5b7237ed7d73874d6ae91f3547)
+++ b/chipper/eslint/.eslintignore	(date 1711136677379)
@@ -11,6 +11,11 @@
 **/images/**/*_jpg.ts
 **/images/**/*_png.ts
 **/sounds/**/*_mp3.ts
+**/mipmaps/**/*_png.js
+**/mipmaps/**/*_png.ts
+**/mipmaps/**/*_jpg.js
+**/mipmaps/**/*_jpg.ts
+
 ../phet-io-website/root/assets/js/jquery-1.12.3.min.js
 ../phet-io-website/root/assets/highlight.js-9.1.0/highlight.pack.js
 ../phet-io-website/root/assets/highlight.js-9.1.0/highlight.js
@@ -29,4 +34,16 @@
 ../scenery/js/display/guillotiere/pkg
 ../alpenglow/doc/lib/
 ../yotta/js/reports/by-simulation.html
-**/*.json
\ No newline at end of file
+**/*.json
+**/*.md
+**/*.pegjs
+**/*_svg.ts
+**/*_mp3.js
+**/*_en.html
+**/*_a11y_view.html
+**/*_wav.js
+**/fenster/**
+**/binder/**
+**/decaf/**
+**/scenery-lab-demo/**
+**/assets/**
\ No newline at end of file

@zepumph and I are optimistic about this approach but will need to schedule time to bring it to production.

[samreid]

Please visit the following issue after this one is complete:

• Lint takes too long and has a memory leak #1415

[samreid]

I just got this error in trying to reproduce the npx command above:

~/phet/root/chipper$ time npx eslint --rulesdir ../chipper/eslint/rules/ --resolve-plugins-relative-to ../chipper --no-error-on-unmatched-pattern --ignore-path ../chipper/eslint/.eslintignore --cache --quiet ../**/js/**

<--- Last few GCs --->

[71716:0x138018000]   254668 ms: Mark-Compact 4027.3 (4134.5) -> 4020.4 (4144.0) MB, 1463.08 / 0.00 ms  (average mu = 0.654, current mu = 0.135) allocation failure; scavenge might not succeed
[71716:0x138018000]   256970 ms: Mark-Compact 4036.2 (4144.0) -> 4028.1 (4151.2) MB, 2285.21 / 0.00 ms  (average mu = 0.405, current mu = 0.007) allocation failure; scavenge might not succeed


<--- JS stacktrace --->

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
 1: 0x104690bf4 node::Abort() [/usr/local/bin/node]
 2: 0x104690ddc node::ModifyCodeGenerationFromStrings(v8::Local<v8::Context>, v8::Local<v8::Value>, bool) [/usr/local/bin/node]
 3: 0x104814da8 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, v8::OOMDetails const&) [/usr/local/bin/node]
 4: 0x1049e96e8 v8::internal::Heap::GarbageCollectionReasonToString(v8::internal::GarbageCollectionReason) [/usr/local/bin/node]
 5: 0x1049e81c4 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/usr/local/bin/node]
 6: 0x1049de9dc v8::internal::HeapAllocator::AllocateRawWithLightRetrySlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/usr/local/bin/node]
 7: 0x1049df23c v8::internal::HeapAllocator::AllocateRawWithRetryOrFailSlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/usr/local/bin/node]
 8: 0x1049c3b04 v8::internal::Factory::AllocateRaw(int, v8::internal::AllocationType, v8::internal::AllocationAlignment) [/usr/local/bin/node]
 9: 0x1049ba0d4 v8::internal::MaybeHandle<v8::internal::SeqOneByteString> v8::internal::FactoryBase<v8::internal::Factory>::NewRawStringWithMap<v8::internal::SeqOneByteString>(int, v8::internal::Map, v8::internal::AllocationType) [/usr/local/bin/node]
10: 0x104df2b40 v8::internal::IncrementalStringBuilder::IncrementalStringBuilder(v8::internal::Isolate*) [/usr/local/bin/node]
11: 0x104af2c28 v8::internal::JsonStringify(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>) [/usr/local/bin/node]
12: 0x1048a32bc v8::internal::Builtin_JsonStringify(int, unsigned long*, v8::internal::Isolate*) [/usr/local/bin/node]
13: 0x105108b24 Builtins_CEntry_Return1_ArgvOnStack_BuiltinExit [/usr/local/bin/node]
14: 0x10aed1a8c 
15: 0x10aed1eec 
16: 0x10aed2014 
17: 0x10aed1eec 
18: 0x10aed1eec 
19: 0x10aed11ac 
20: 0x1050803e4 Builtins_InterpreterEntryTrampoline [/usr/local/bin/node]
21: 0x1050803e4 Builtins_InterpreterEntryTrampoline [/usr/local/bin/node]
22: 0x105164fb8 Builtins_PromiseFulfillReactionJob [/usr/local/bin/node]
23: 0x1050a6b94 Builtins_RunMicrotasks [/usr/local/bin/node]
24: 0x10507e3f4 Builtins_JSRunMicrotasksEntry [/usr/local/bin/node]
25: 0x1049569f0 v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [/usr/local/bin/node]
26: 0x104956edc v8::internal::(anonymous namespace)::InvokeWithTryCatch(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [/usr/local/bin/node]
27: 0x1049570b8 v8::internal::Execution::TryRunMicrotasks(v8::internal::Isolate*, v8::internal::MicrotaskQueue*) [/usr/local/bin/node]
28: 0x10497e284 v8::internal::MicrotaskQueue::RunMicrotasks(v8::internal::Isolate*) [/usr/local/bin/node]
29: 0x10497ea20 v8::internal::MicrotaskQueue::PerformCheckpoint(v8::Isolate*) [/usr/local/bin/node]
30: 0x1045c0c64 node::InternalCallbackScope::Close() [/usr/local/bin/node]
31: 0x1045c101c node::InternalMakeCallback(node::Environment*, v8::Local<v8::Object>, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) [/usr/local/bin/node]
32: 0x1045d748c node::AsyncWrap::MakeCallback(v8::Local<v8::Function>, int, v8::Local<v8::Value>*) [/usr/local/bin/node]
33: 0x1046962a8 node::fs::FSReqCallback::Reject(v8::Local<v8::Value>) [/usr/local/bin/node]
34: 0x104696b84 node::fs::FSReqAfterScope::Reject(uv_fs_s*) [/usr/local/bin/node]
35: 0x104696fa4 node::fs::AfterStat(uv_fs_s*) [/usr/local/bin/node]
36: 0x10468bcf0 node::MakeLibuvRequestCallback<uv_fs_s, void (*)(uv_fs_s*)>::Wrapper(uv_fs_s*) [/usr/local/bin/node]
37: 0x10505cb64 uv__work_done [/usr/local/bin/node]
38: 0x1050605b4 uv__async_io [/usr/local/bin/node]
39: 0x10507268c uv__io_poll [/usr/local/bin/node]
40: 0x105060b78 uv_run [/usr/local/bin/node]
41: 0x1045c1754 node::SpinEventLoopInternal(node::Environment*) [/usr/local/bin/node]
42: 0x1046d08d8 node::NodeMainInstance::Run(node::ExitCode*, node::Environment*) [/usr/local/bin/node]
43: 0x1046d0674 node::NodeMainInstance::Run() [/usr/local/bin/node]
44: 0x10465b030 node::Start(int, char**) [/usr/local/bin/node]
45: 0x18905d0e0 start [/usr/lib/dyld]
Abort trap: 6

real	4m17.294s
user	5m59.269s
sys	0m37.121s

But I likely have other repos sneaking in.

[zepumph]

Good progress for 10 minutes with @samreid just now. This newest patch is our first run on windows, and it is working well!

• platform check for npx.cmd (just like gruntCommand)
• Revert changes in perennial-alias, as we don't want to commit to both copies.

Subject: [PATCH] doc update, https://github.com/phetsims/phet-io-wrappers/issues/637
---
Index: chipper/eslint/.eslintignore
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/eslint/.eslintignore b/chipper/eslint/.eslintignore
--- a/chipper/eslint/.eslintignore	(revision f158a3bd7f490a94c3fd577a3cea9c953cbfbde2)
+++ b/chipper/eslint/.eslintignore	(date 1712265583645)
@@ -11,6 +11,11 @@
 **/images/**/*_jpg.ts
 **/images/**/*_png.ts
 **/sounds/**/*_mp3.ts
+**/mipmaps/**/*_png.js
+**/mipmaps/**/*_png.ts
+**/mipmaps/**/*_jpg.js
+**/mipmaps/**/*_jpg.ts
+
 ../phet-io-website/root/assets/js/jquery-1.12.3.min.js
 ../phet-io-website/root/assets/highlight.js-9.1.0/highlight.pack.js
 ../phet-io-website/root/assets/highlight.js-9.1.0/highlight.js
@@ -29,4 +34,15 @@
 ../scenery/js/display/guillotiere/pkg
 ../alpenglow/doc/lib/
 ../yotta/js/reports/by-simulation.html
-**/*.json
\ No newline at end of file
+**/*.json
+**/*.md
+**/*.pegjs
+**/*_svg.ts
+**/*_mp3.js
+**/*_en.html
+**/*_a11y_view.html
+**/*_wav.js
+**/fenster/**
+**/decaf/**
+**/scenery-lab-demo/**
+**/assets/**
\ No newline at end of file
Index: chipper/js/grunt/Gruntfile.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/grunt/Gruntfile.js b/chipper/js/grunt/Gruntfile.js
--- a/chipper/js/grunt/Gruntfile.js	(revision f158a3bd7f490a94c3fd577a3cea9c953cbfbde2)
+++ b/chipper/js/grunt/Gruntfile.js	(date 1712265492492)
@@ -349,7 +349,7 @@
 --disable-with-comment: add an es-lint disable with comment to lint errors
 --repos: comma separated list of repos to lint in addition to the repo from running`,
     wrapTask( async () => {
-      const lint = require( './lint' );
+      const npxLint = require( './npxLint' );
 
       // --disable-eslint-cache disables the cache, useful for developing rules
       const cache = !grunt.option( 'disable-eslint-cache' );
@@ -359,7 +359,7 @@
 
       const extraRepos = grunt.option( 'repos' ) ? grunt.option( 'repos' ).split( ',' ) : [];
 
-      const lintReturnValue = await lint( [ repo, ...extraRepos ], {
+      const lintReturnValue = await npxLint( [ repo, ...extraRepos ], {
         cache: cache,
         fix: fix,
         chipAway: chipAway,
@@ -372,7 +372,7 @@
     } ) );
 
   grunt.registerTask( 'lint-all', 'lint all js files that are required to build this repository (for the specified brands)', wrapTask( async () => {
-    const lint = require( './lint' );
+    const npxLint = require( './npxLint' );
 
     // --disable-eslint-cache disables the cache, useful for developing rules
     const cache = !grunt.option( 'disable-eslint-cache' );
@@ -385,7 +385,7 @@
 
     const brands = getBrands( grunt, repo, buildLocal );
 
-    const lintReturnValue = await lint( getPhetLibs( repo, brands ), {
+    const lintReturnValue = await npxLint( getPhetLibs( repo, brands ), {
       cache: cache,
       fix: fix,
       chipAway: chipAway,
Index: perennial/js/grunt/Gruntfile.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/perennial/js/grunt/Gruntfile.js b/perennial/js/grunt/Gruntfile.js
--- a/perennial/js/grunt/Gruntfile.js	(revision 89a59a976596404533605544ed5485aa4b4165e8)
+++ b/perennial/js/grunt/Gruntfile.js	(date 1712265492501)
@@ -537,8 +537,9 @@
 
     // Don't always require this, as we may have an older chipper checked out.  Also make sure it is the promise-based lint.
     const lint = require( '../../../chipper/js/grunt/lint' );
-    if ( lint.chipperAPIVersion === 'promisesPerRepo1' ) {
-      const lintReturnValue = await lint( activeRepos, {
+    const npxLint = require( '../../../chipper/js/grunt/npxLint' );
+    if ( lint.chipperAPIVersion === 'promisesPerRepo1' && npxLint ) {
+      const lintReturnValue = await npxLint( activeRepos, {
         cache: cache,
         fix: fix,
         chipAway: chipAway,
Index: chipper/js/scripts/hook-pre-commit-task.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/scripts/hook-pre-commit-task.js b/chipper/js/scripts/hook-pre-commit-task.js
--- a/chipper/js/scripts/hook-pre-commit-task.js	(revision f158a3bd7f490a94c3fd577a3cea9c953cbfbde2)
+++ b/chipper/js/scripts/hook-pre-commit-task.js	(date 1712265492496)
@@ -16,7 +16,7 @@
 const generatePhetioMacroAPI = require( '../phet-io/generatePhetioMacroAPI' );
 const CacheLayer = require( '../../../chipper/js/common/CacheLayer' );
 const phetioCompareAPISets = require( '../phet-io/phetioCompareAPISets' );
-const lint = require( '../../../chipper/js/grunt/lint' );
+const npxLint = require( '../../../chipper/js/grunt/npxLint' );
 const reportMedia = require( '../../../chipper/js/grunt/reportMedia' );
 const puppeteerQUnit = require( '../../../aqua/js/local/puppeteerQUnit' );
 const Transpiler = require( '../../../chipper/js/common/Transpiler' );
@@ -43,7 +43,7 @@
 
     // Run lint tests if they exist in the checked-out SHAs.
     // lint() automatically filters out non-lintable repos
-    const lintReturnValue = await lint( [ repo ] );
+    const lintReturnValue = await npxLint( [ repo ] );
     outputToConsole && console.log( `Linting passed with results.length: ${lintReturnValue.results.length}` );
     process.exit( lintReturnValue.ok ? 0 : 1 );
   }
Index: chipper/js/grunt/npxLint.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/grunt/npxLint.js b/chipper/js/grunt/npxLint.js
--- a/chipper/js/grunt/npxLint.js	(revision f158a3bd7f490a94c3fd577a3cea9c953cbfbde2)
+++ b/chipper/js/grunt/npxLint.js	(date 1712266337364)
@@ -29,7 +29,7 @@
 
     console.log( `running in cwd ../chipper: npx ${args.join( ' ' )}` );
 
-    const eslint = spawn( 'npx', args, {
+    const eslint = spawn( /^win/.test( process.platform ) ? 'npx.cmd' : 'npx', args, {
       cwd: '../chipper'
     } );
 

[samreid]

Building on the patch above, I added --fix and --disable-eslint-cache options. I tested each and they worked well.

Subject: [PATCH] Add npx.cmd, options.cache, and options.fix, see https://github.com/phetsims/chipper/issues/1429
---
Index: chipper/js/grunt/Gruntfile.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/grunt/Gruntfile.js b/chipper/js/grunt/Gruntfile.js
--- a/chipper/js/grunt/Gruntfile.js	(revision aba682fffe1f05b2f2df65781e23123047653db4)
+++ b/chipper/js/grunt/Gruntfile.js	(date 1712269024399)
@@ -349,7 +349,7 @@
 --disable-with-comment: add an es-lint disable with comment to lint errors
 --repos: comma separated list of repos to lint in addition to the repo from running`,
     wrapTask( async () => {
-      const lint = require( './lint' );
+      const npxLint = require( './npxLint' );
 
       // --disable-eslint-cache disables the cache, useful for developing rules
       const cache = !grunt.option( 'disable-eslint-cache' );
@@ -359,7 +359,7 @@
 
       const extraRepos = grunt.option( 'repos' ) ? grunt.option( 'repos' ).split( ',' ) : [];
 
-      const lintReturnValue = await lint( [ repo, ...extraRepos ], {
+      const lintReturnValue = await npxLint( [ repo, ...extraRepos ], {
         cache: cache,
         fix: fix,
         chipAway: chipAway,
@@ -372,7 +372,7 @@
     } ) );
 
   grunt.registerTask( 'lint-all', 'lint all js files that are required to build this repository (for the specified brands)', wrapTask( async () => {
-    const lint = require( './lint' );
+    const npxLint = require( './npxLint' );
 
     // --disable-eslint-cache disables the cache, useful for developing rules
     const cache = !grunt.option( 'disable-eslint-cache' );
@@ -385,7 +385,7 @@
 
     const brands = getBrands( grunt, repo, buildLocal );
 
-    const lintReturnValue = await lint( getPhetLibs( repo, brands ), {
+    const lintReturnValue = await npxLint( getPhetLibs( repo, brands ), {
       cache: cache,
       fix: fix,
       chipAway: chipAway,
Index: perennial/js/grunt/Gruntfile.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/perennial/js/grunt/Gruntfile.js b/perennial/js/grunt/Gruntfile.js
--- a/perennial/js/grunt/Gruntfile.js	(revision c95e245cfa2f79680e406a2d636cf21eabfdfd20)
+++ b/perennial/js/grunt/Gruntfile.js	(date 1712269024413)
@@ -537,8 +537,9 @@
 
     // Don't always require this, as we may have an older chipper checked out.  Also make sure it is the promise-based lint.
     const lint = require( '../../../chipper/js/grunt/lint' );
-    if ( lint.chipperAPIVersion === 'promisesPerRepo1' ) {
-      const lintReturnValue = await lint( activeRepos, {
+    const npxLint = require( '../../../chipper/js/grunt/npxLint' );
+    if ( lint.chipperAPIVersion === 'promisesPerRepo1' && npxLint ) {
+      const lintReturnValue = await npxLint( activeRepos, {
         cache: cache,
         fix: fix,
         chipAway: chipAway,
Index: chipper/js/scripts/hook-pre-commit-task.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/scripts/hook-pre-commit-task.js b/chipper/js/scripts/hook-pre-commit-task.js
--- a/chipper/js/scripts/hook-pre-commit-task.js	(revision aba682fffe1f05b2f2df65781e23123047653db4)
+++ b/chipper/js/scripts/hook-pre-commit-task.js	(date 1712269024408)
@@ -16,7 +16,7 @@
 const generatePhetioMacroAPI = require( '../phet-io/generatePhetioMacroAPI' );
 const CacheLayer = require( '../../../chipper/js/common/CacheLayer' );
 const phetioCompareAPISets = require( '../phet-io/phetioCompareAPISets' );
-const lint = require( '../../../chipper/js/grunt/lint' );
+const npxLint = require( '../../../chipper/js/grunt/npxLint' );
 const reportMedia = require( '../../../chipper/js/grunt/reportMedia' );
 const puppeteerQUnit = require( '../../../aqua/js/local/puppeteerQUnit' );
 const Transpiler = require( '../../../chipper/js/common/Transpiler' );
@@ -43,7 +43,7 @@
 
     // Run lint tests if they exist in the checked-out SHAs.
     // lint() automatically filters out non-lintable repos
-    const lintReturnValue = await lint( [ repo ] );
+    const lintReturnValue = await npxLint( [ repo ] );
     outputToConsole && console.log( `Linting passed with results.length: ${lintReturnValue.results.length}` );
     process.exit( lintReturnValue.ok ? 0 : 1 );
   }
Index: chipper/js/grunt/npxLint.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/grunt/npxLint.js b/chipper/js/grunt/npxLint.js
--- a/chipper/js/grunt/npxLint.js	(revision aba682fffe1f05b2f2df65781e23123047653db4)
+++ b/chipper/js/grunt/npxLint.js	(date 1712270910800)
@@ -9,15 +9,35 @@
 
 // modules
 const { spawn } = require( 'child_process' ); // eslint-disable-line require-statement-match
+const _ = require( 'lodash' );
+
+function runEslint( repos, options ) {
 
-function runEslint( repos ) {
+  options = _.assignIn( {
+
+    // Cache results for a speed boost
+    cache: true,
+
+    // Fix things that can be autofixed
+    fix: false
+  }, options );
 
   const patterns = repos.map( repo => `../${repo}/` );
   return new Promise( ( resolve, reject ) => {
-    const args = [
-      'eslint',
-      '--cache',
-      '--cache-location', '../chipper/eslint/cache/.eslintcache',
+    const args = [ 'eslint' ];
+
+    // Conditionally add cache options based on the cache flag in options
+    if ( options.cache ) {
+      args.push( '--cache', '--cache-location', '../chipper/eslint/cache/.eslintcache' );
+    }
+
+    // Add the '--fix' option if fix is true
+    if ( options.fix ) {
+      args.push( '--fix' );
+    }
+
+    // Continue building the args array
+    args.push( ...[
       '--rulesdir', '../chipper/eslint/rules/',
       '--resolve-plugins-relative-to', '../chipper',
       '--no-error-on-unmatched-pattern',
@@ -25,11 +45,11 @@
       '--ext', '.js,.jsx,.ts,.tsx,.mjs,.cjs,.html',
       '--quiet',
       ...patterns
-    ];
+    ] );
 
     console.log( `running in cwd ../chipper: npx ${args.join( ' ' )}` );
 
-    const eslint = spawn( 'npx', args, {
+    const eslint = spawn( /^win/.test( process.platform ) ? 'npx.cmd' : 'npx', args, {
       cwd: '../chipper'
     } );
 
@@ -61,7 +81,7 @@
  */
 const npxLint = async ( originalRepos, options ) => {
   try {
-    const result = await runEslint( originalRepos );
+    const result = await runEslint( originalRepos, options );
     console.log( 'ESLint output:', result );
     if ( result.trim().length === 0 ) {
       return { results: [], ok: true };
Index: chipper/eslint/.eslintignore
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/eslint/.eslintignore b/chipper/eslint/.eslintignore
--- a/chipper/eslint/.eslintignore	(revision aba682fffe1f05b2f2df65781e23123047653db4)
+++ b/chipper/eslint/.eslintignore	(date 1712269024392)
@@ -11,6 +11,11 @@
 **/images/**/*_jpg.ts
 **/images/**/*_png.ts
 **/sounds/**/*_mp3.ts
+**/mipmaps/**/*_png.js
+**/mipmaps/**/*_png.ts
+**/mipmaps/**/*_jpg.js
+**/mipmaps/**/*_jpg.ts
+
 ../phet-io-website/root/assets/js/jquery-1.12.3.min.js
 ../phet-io-website/root/assets/highlight.js-9.1.0/highlight.pack.js
 ../phet-io-website/root/assets/highlight.js-9.1.0/highlight.js
@@ -29,4 +34,15 @@
 ../scenery/js/display/guillotiere/pkg
 ../alpenglow/doc/lib/
 ../yotta/js/reports/by-simulation.html
-**/*.json
\ No newline at end of file
+**/*.json
+**/*.md
+**/*.pegjs
+**/*_svg.ts
+**/*_mp3.js
+**/*_en.html
+**/*_a11y_view.html
+**/*_wav.js
+**/fenster/**
+**/decaf/**
+**/scenery-lab-demo/**
+**/assets/**
\ No newline at end of file

Other options we can decide whether to support or not:

• chipAway
• disableWithComment
• showProgressBar

[samreid]

Here's a makeshift progress bar.

    // Prepare environment for spawn process
    const env = Object.create(process.env);
    if (options.showProgressBar) {
      env.DEBUG = 'eslint:cli-engine';
    }

    const eslint = spawn(/^win/.test(process.platform) ? 'npx.cmd' : 'npx', args, {
      cwd: '../chipper',
      env: env // Use the prepared environment
    });

But maybe it would be better to omit altogether.

When is the last time we used chipAway? Do we need that any more? Or can we add it if/when needed?
When is the last time we used disableWithComment? Do we need that any more? I hope not.

[samreid]

Should we add on npx --node-options='--max-old-space-size=8192' to the execution call?

[zepumph]

I did some more work here, and I believe that it would be best to discuss in person before too long.

1. I think we should delete disableWithComment, and the file too
2. I want to keep chipAway, but it seems hard, let's brainstorm.
3. I added the node option
4. I committed the support for a progress bar, but testing on a mac sounds important since there is some file path stuff.
5. I added many TODOs to aid us in getting this to production.

Subject: [PATCH] update doc, https://github.com/phetsims/chipper/issues/1429
---
Index: chipper/js/scripts/hook-pre-commit-task.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/scripts/hook-pre-commit-task.js b/chipper/js/scripts/hook-pre-commit-task.js
--- a/chipper/js/scripts/hook-pre-commit-task.js	(revision 392601ebc89317087297fb34d91fcd55b34c5549)
+++ b/chipper/js/scripts/hook-pre-commit-task.js	(date 1712594936795)
@@ -16,7 +16,7 @@
 const generatePhetioMacroAPI = require( '../phet-io/generatePhetioMacroAPI' );
 const CacheLayer = require( '../../../chipper/js/common/CacheLayer' );
 const phetioCompareAPISets = require( '../phet-io/phetioCompareAPISets' );
-const lint = require( '../../../chipper/js/grunt/lint' );
+const npxLint = require( '../../../chipper/js/grunt/npxLint' );
 const reportMedia = require( '../../../chipper/js/grunt/reportMedia' );
 const puppeteerQUnit = require( '../../../aqua/js/local/puppeteerQUnit' );
 const Transpiler = require( '../../../chipper/js/common/Transpiler' );
@@ -43,7 +43,7 @@
 
     // Run lint tests if they exist in the checked-out SHAs.
     // lint() automatically filters out non-lintable repos
-    const lintReturnValue = await lint( [ repo ] );
+    const lintReturnValue = await npxLint( [ repo ] );
     outputToConsole && console.log( `Linting passed with results.length: ${lintReturnValue.results.length}` );
     process.exit( lintReturnValue.ok ? 0 : 1 );
   }
Index: chipper/eslint/.eslintignore
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/eslint/.eslintignore b/chipper/eslint/.eslintignore
--- a/chipper/eslint/.eslintignore	(revision 392601ebc89317087297fb34d91fcd55b34c5549)
+++ b/chipper/eslint/.eslintignore	(date 1712594936785)
@@ -11,6 +11,11 @@
 **/images/**/*_jpg.ts
 **/images/**/*_png.ts
 **/sounds/**/*_mp3.ts
+**/mipmaps/**/*_png.js
+**/mipmaps/**/*_png.ts
+**/mipmaps/**/*_jpg.js
+**/mipmaps/**/*_jpg.ts
+
 ../phet-io-website/root/assets/js/jquery-1.12.3.min.js
 ../phet-io-website/root/assets/highlight.js-9.1.0/highlight.pack.js
 ../phet-io-website/root/assets/highlight.js-9.1.0/highlight.js
@@ -29,4 +34,15 @@
 ../scenery/js/display/guillotiere/pkg
 ../alpenglow/doc/lib/
 ../yotta/js/reports/by-simulation.html
-**/*.json
\ No newline at end of file
+**/*.json
+**/*.md
+**/*.pegjs
+**/*_svg.ts
+**/*_mp3.js
+**/*_en.html
+**/*_a11y_view.html
+**/*_wav.js
+**/fenster/**
+**/decaf/**
+**/scenery-lab-demo/**
+**/assets/**
\ No newline at end of file
Index: perennial/js/grunt/Gruntfile.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/perennial/js/grunt/Gruntfile.js b/perennial/js/grunt/Gruntfile.js
--- a/perennial/js/grunt/Gruntfile.js	(revision 89a59a976596404533605544ed5485aa4b4165e8)
+++ b/perennial/js/grunt/Gruntfile.js	(date 1712599843721)
@@ -535,10 +535,12 @@
     const disableWithComment = grunt.option( 'disable-with-comment' );
     const showProgressBar = !grunt.option( 'hide-progress-bar' );
 
+    // TODO: only need one of these, https://github.com/phetsims/chipper/issues/1429
     // Don't always require this, as we may have an older chipper checked out.  Also make sure it is the promise-based lint.
     const lint = require( '../../../chipper/js/grunt/lint' );
-    if ( lint.chipperAPIVersion === 'promisesPerRepo1' ) {
-      const lintReturnValue = await lint( activeRepos, {
+    const npxLint = require( '../../../chipper/js/grunt/npxLint' );
+    if ( lint.chipperAPIVersion === 'promisesPerRepo1' && npxLint ) {
+      const lintReturnValue = await npxLint( activeRepos, {
         cache: cache,
         fix: fix,
         chipAway: chipAway,
Index: chipper/js/grunt/Gruntfile.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/grunt/Gruntfile.js b/chipper/js/grunt/Gruntfile.js
--- a/chipper/js/grunt/Gruntfile.js	(revision 392601ebc89317087297fb34d91fcd55b34c5549)
+++ b/chipper/js/grunt/Gruntfile.js	(date 1712599843728)
@@ -349,7 +349,7 @@
 --disable-with-comment: add an es-lint disable with comment to lint errors
 --repos: comma separated list of repos to lint in addition to the repo from running`,
     wrapTask( async () => {
-      const lint = require( './lint' );
+      const npxLint = require( './npxLint' );
 
       // --disable-eslint-cache disables the cache, useful for developing rules
       const cache = !grunt.option( 'disable-eslint-cache' );
@@ -359,7 +359,7 @@
 
       const extraRepos = grunt.option( 'repos' ) ? grunt.option( 'repos' ).split( ',' ) : [];
 
-      const lintReturnValue = await lint( [ repo, ...extraRepos ], {
+      const lintReturnValue = await npxLint( [ repo, ...extraRepos ], {
         cache: cache,
         fix: fix,
         chipAway: chipAway,
@@ -372,7 +372,7 @@
     } ) );
 
   grunt.registerTask( 'lint-all', 'lint all js files that are required to build this repository (for the specified brands)', wrapTask( async () => {
-    const lint = require( './lint' );
+    const npxLint = require( './npxLint' );
 
     // --disable-eslint-cache disables the cache, useful for developing rules
     const cache = !grunt.option( 'disable-eslint-cache' );
@@ -385,7 +385,7 @@
 
     const brands = getBrands( grunt, repo, buildLocal );
 
-    const lintReturnValue = await lint( getPhetLibs( repo, brands ), {
+    const lintReturnValue = await npxLint( getPhetLibs( repo, brands ), {
       cache: cache,
       fix: fix,
       chipAway: chipAway,

[zepumph]

Lots more progress this afternoon. I added chip-away, which involved using the json output for linting. I don't know if we will want this forever, since it saves all the output until the end of the process (so grunt lint-everything doesn't give in-progress results). We should discuss more.

[zepumph]

Alright. I finally was able to push the full conversion. Main is now running npx linting and the old lint file was deleted. We also found that we had made a mistake. NPX lint does not give out results as it runs ever, so we were able to go with JSON output easier, because we weren't giving up result streaming. I'll check in with CT. @samreid can you take a look at the changes (by looking at the new and improved lint.js file). What's next?

[zepumph]

Oh man. CTQ had a few things to say about how we were doing the logging parsing. The JSON output isn't always in a single chunk. I believe we are doing much better, and I gave the file another once over. Please take it from here.

[samreid]

I reviewed the commits above. CT seems stable. I do not see any lint-related issues on CT at the moment. I believe we are in good shape to close this issue. Nice work @zepumph

[samreid]

We discovered that --disable-eslint-cache does't clear a (say, bad) cache. I'll propose a fix

[samreid]

Additionally, we believe it would be preferable to always write to the cache, even on --disable-eslint-cache so that on the next run the cache can be consulted. IMO this is the preferable behavior and the only downside is that it doesn't match the semantics of "disabling the cache" well. Still, I think we should commit it and ask @zepumph for review.

[zepumph]

I love these commits, and for a while I believe that --disable-eslint-cache has been a misnomer. Really we want to run it as a --clean step. I don't recommend changing the option, but I'll update doc. Thanks!