phetsims / chipper

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

Do we need to turn off js rule to turn on typescript-eslint rule? #1495

Closed zepumph closed 1 month ago

zepumph commented 1 month ago

Promoting TODO:

 // You must disable the base rule to avoid duplicate/incorrect errors. TODO: Is that still necessary?
. . . 

      // Enforce default parameters to be last
      'default-param-last': 'off',
      '@typescript-eslint/default-param-last': 'error',

      // Enforce dot notation whenever possible 🔒 🔧 💭
      'dot-notation': 'off',
      '@typescript-eslint/dot-notation': 'error',
. . . 
samreid commented 1 month ago

Yes, it is still necessary to turn off the JS rule when turning on the typescript-eslint-rule. For example:


Subject: [PATCH] Copy localeData.json from main to tests branch, see https://github.com/phetsims/perennial/issues/385
---
Index: js/density-main.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/density-main.ts b/js/density-main.ts
--- a/js/density-main.ts    (revision 8797c346f72d72079779cc91484a072fad254bd8)
+++ b/js/density-main.ts    (date 1730119720473)
@@ -59,7 +59,7 @@
   ], simOptions );

   DensityBuoyancyCommonQueryParameters.descriptionPrototype && sim.isConstructionCompleteProperty.lazyLink( isConstructionComplete => {
-    DescriptionContext.startupComplete();
+    DescriptionContext['startupComplete']();
   } );
   sim.start();
 } );
\ No newline at end of file

yields:

~/phet/root$ cd density
~/phet/root/density$ grunt lint
Running "lint" task
(Forwarding task to perennial-alias)
Running "lint" task

density:

/Users/samreid/phet/root/density/js/density-main.ts
  62:23  error  A space is required after '['                          computed-property-spacing
  62:24  error  ["startupComplete"] is better written in dot notation  dot-notation
  62:24  error  ["startupComplete"] is better written in dot notation  @typescript-eslint/dot-notation
  62:41  error  A space is required before ']'                         computed-property-spacing

✖ 4 problems (4 errors, 0 warnings)
  4 errors and 0 warnings potentially fixable with the `--fix` option.

Fatal error: Lint failed
Fatal error: spawn: /Users/samreid/phet/root/perennial-alias/node_modules/.bin/tsx "/Users/samreid/phet/root/perennial-alias/js/grunt/tasks/lint.ts" "--repo=density" failed with code 1
Fatal error: spawn: grunt "lint" "--repo=density" failed with code 3

Note that dot-notation and typescript-eslint/dot-notation redundandtly report.

samreid commented 1 month ago

I removed the TODO, but I should bring back the explanatory part of it, when my working copy is clearer.

samreid commented 1 month ago

Fixed, closing.

zepumph commented 1 month ago

Thank you!