phetsims / chipper

Tools for developing and building PhET interactive simulations.
http://scenerystack.org/
MIT License
12 stars 14 forks source link

Update lint.js now that we are using ESLint 9 #1484

Closed samreid closed 2 weeks ago

samreid commented 1 month ago

From https://github.com/phetsims/chipper/issues/1451 there were many TODOs to upgrade the lint process. Most of this should be done before #1415.

zepumph commented 1 month ago

From https://github.com/phetsims/chipper/issues/1469 here are some checklist items:

samreid commented 3 weeks ago

I'll run performance tests of using the API or processes on cached or --clean, on mac and windows. That will help us know for sure whether we need both implementations.

samreid commented 3 weeks ago
mac
cache is fresh - API only => 9sec
cache is fresh - spawn only => 52sec
--clean - spawn only => 4m22s
--clean - API only => 54% then crash at 2m40s

windows
cache is fresh - API only => 16sec
cache is fresh - spawn only => 63s
--clean - spawn only => 4m23s
--clean - API only => 54% then crash at 4m14s
zepumph commented 3 weeks ago

Design goals:

  1. No crashing possible
  2. Cleaner implementation than one file with two implementations

Next steps:

  1. Let's go all in on the Node CLI (API), and use sub processes to keep heaps for overloading. Lets start with 50 repos per sub process.
  2. We can run multiple sub processes at once (multi threading)
  3. We can remove the npx implementation
  4. Keep the CLI running a single repo at a time, with a cache file per repo.
zepumph commented 3 weeks ago

Subject: [PATCH] add authors, https://github.com/phetsims/chipper/issues/1502
---
Index: js/grunt/lint.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/grunt/lint.ts b/js/grunt/lint.ts
--- a/js/grunt/lint.ts  (revision 9c6461e0c47a003490903950a5c25b4448071083)
+++ b/js/grunt/lint.ts  (date 1730747840002)
@@ -175,7 +175,8 @@
     cache: true,
     cacheLocation: path.resolve( getCacheLocation( repo ) ),
     fix: options.fix,
-    errorOnUnmatchedPattern: false
+    errorOnUnmatchedPattern: false,
+    flags: [ '--debug' ]
   };

   // Create ESLint instance
samreid commented 2 weeks ago
samreid commented 2 weeks ago

Ready for review in the chipper and perennial-alias branches lint-batches-chipper-1484. Note there is one remaining TODO that I could not figure out how to solve and need help on.

zepumph commented 2 weeks ago
zepumph commented 2 weeks ago

 mjkauzmann ~/PHET/git/perennial-alias (lint-batches-chipper-1484)
 $ time grunt lint --all --clean
Running "lint" task
processes 4
Done.

real    2m18.486s
user    0m0.061s
sys     0m0.061s

 mjkauzmann ~/PHET/git/perennial-alias (lint-batches-chipper-1484)
 $ time grunt lint --all --clean
Running "lint" task
processes 15
Done.

real    1m56.170s
user    0m0.060s
sys     0m0.062s
zepumph commented 2 weeks ago

I added 6 TODOs as part of the review. @samreid and I merged to main, and the rest is in his court. Thanks SR!1

samreid commented 2 weeks ago

TODOs addressed, --processes added which sets the maximum number of allowable processes. We are merged to main. Feature branches deleted. Closing.

samreid commented 2 weeks ago

On second thought, I'm very confused about all the entry points that are allowed to pass through LintOptions. The default occurs in several places. Would be good for @zepumph to review and comment.

zepumph commented 2 weeks ago

@samreid and I agree it is awkward, and getLintOptions shouldn't always provide defaults that are duplicated with the module defaults. We will make a separate side issue.