eslint should lint javascript in html <script> tags

[zepumph]

With a very small patch, we can lint our js into HTML. The issue will be handling the lint errors.

https://www.npmjs.com/package/eslint-plugin-html

Subject: [PATCH] move out html from this file (preserving history) (WORKING), https://github.com/phetsims/phet-io-wrappers/issues/540
---
Index: js/grunt/lint.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/grunt/lint.js b/js/grunt/lint.js
--- a/js/grunt/lint.js	(revision 2ba9214be503ebc571645ffda5c150a1d26c68b4)
+++ b/js/grunt/lint.js	(date 1695834656905)
@@ -80,7 +80,7 @@
     // Our custom rules live here
     rulePaths: [ '../chipper/eslint/rules' ],
 
-    extensions: [ '.js', '.jsx', '.ts', '.tsx', '.mjs', '.cjs' ],
+    extensions: [ '.js', '.jsx', '.ts', '.tsx', '.mjs', '.cjs', '.html' ],
 
     // If no lintable files are found, it is not an error
     errorOnUnmatchedPattern: false
Index: eslint/.eslintrc.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/eslint/.eslintrc.js b/eslint/.eslintrc.js
--- a/eslint/.eslintrc.js	(revision 2ba9214be503ebc571645ffda5c150a1d26c68b4)
+++ b/eslint/.eslintrc.js	(date 1695834564333)
@@ -16,6 +16,9 @@
   // Without a parser, .js files are linted without es6 transpilation. Use the same parser that we use for TypeScript.
   parser: '@typescript-eslint/parser',
 
+  // Lint javascript in HTML files too
+  plugins: [ 'html' ],
+
   overrides: [
     {
 
Index: package.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/package.json b/package.json
--- a/package.json	(revision 2ba9214be503ebc571645ffda5c150a1d26c68b4)
+++ b/package.json	(date 1695834389552)
@@ -37,6 +37,7 @@
     "docdash": "~1.2.0",
     "eslint": "~8.28.0",
     "eslint-plugin-react": "~7.31.11",
+    "eslint-plugin-html": "~7.1.0",
     "grunt": "~1.5.3",
     "html-webpack-plugin": "~5.3.2",
     "jimp": "~0.16.1",

[zepumph]

We need to handle disabling eslint when we copy the package.json.

Subject: [PATCH] so hard, so bad
---
Index: js/grunt/lint.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/grunt/lint.js b/js/grunt/lint.js
--- a/js/grunt/lint.js	(revision 2ba9214be503ebc571645ffda5c150a1d26c68b4)
+++ b/js/grunt/lint.js	(date 1695934812770)
@@ -80,7 +80,7 @@
     // Our custom rules live here
     rulePaths: [ '../chipper/eslint/rules' ],
 
-    extensions: [ '.js', '.jsx', '.ts', '.tsx', '.mjs', '.cjs' ],
+    extensions: [ '.js', '.jsx', '.ts', '.tsx', '.mjs', '.cjs', '.html' ],
 
     // If no lintable files are found, it is not an error
     errorOnUnmatchedPattern: false
Index: eslint/.eslintrc.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/eslint/.eslintrc.js b/eslint/.eslintrc.js
--- a/eslint/.eslintrc.js	(revision 2ba9214be503ebc571645ffda5c150a1d26c68b4)
+++ b/eslint/.eslintrc.js	(date 1695934980483)
@@ -16,6 +16,9 @@
   // Without a parser, .js files are linted without es6 transpilation. Use the same parser that we use for TypeScript.
   parser: '@typescript-eslint/parser',
 
+  // Lint javascript in HTML files too
+  plugins: [ 'html' ],
+
   overrides: [
     {
 
Index: eslint/.eslintignore
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/eslint/.eslintignore b/eslint/.eslintignore
--- a/eslint/.eslintignore	(revision 2ba9214be503ebc571645ffda5c150a1d26c68b4)
+++ b/eslint/.eslintignore	(date 1695935029780)
@@ -5,6 +5,7 @@
 **/snapshots
 **/js/parser/svgPath.js
 **/templates/*.js
+**/templates/*.html
 **/js/**/*Strings.ts
 **/images/**/*_cur.ts
 **/images/**/*_jpg.ts
Index: package.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/package.json b/package.json
--- a/package.json	(revision 2ba9214be503ebc571645ffda5c150a1d26c68b4)
+++ b/package.json	(date 1695934812773)
@@ -37,6 +37,7 @@
     "docdash": "~1.2.0",
     "eslint": "~8.28.0",
     "eslint-plugin-react": "~7.31.11",
+    "eslint-plugin-html": "~7.1.0",
     "grunt": "~1.5.3",
     "html-webpack-plugin": "~5.3.2",
     "jimp": "~0.16.1",
Index: eslint/rules/copyright.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/eslint/rules/copyright.js b/eslint/rules/copyright.js
--- a/eslint/rules/copyright.js	(revision 2ba9214be503ebc571645ffda5c150a1d26c68b4)
+++ b/eslint/rules/copyright.js	(date 1695935151301)
@@ -18,6 +18,12 @@
       // Get the whole source code, not for node only.
       const comments = context.getSourceCode().getAllComments();
 
+      const isHTML = context.getFilename().endsWith( '.html' );
+
+      if ( isHTML ) {
+        return;
+      }
+
       if ( !comments || comments.length === 0 ) {
         context.report( {
           node: node,

[zepumph]

In #1408 I updated dev HTML and a11y view files to pass our lint (by ignoring double quotes that will occur when copying the packagejson into the file. That can be committed before turning on lint generally.

[zepumph]

After #1408 I saw an infinite loop occurring in fenster/. After removing that from the list, here is the report:

✖ 17935 problems (17933 errors, 2 warnings)
  15291 errors and 2 warnings potentially fixable with the `--fix` option.


Results from chipAway: 
 - [ ] alpenglow: @jonathanolson 6329 errors in 2 files.
 - [ ] aqua: @zepumph 82 errors in 3 files.
 - [ ] blast: @samreid 8 errors in 1 files.
 - [ ] decaf: @samreid 1 errors in 1 files.
 - [ ] dot: @jonathanolson 82 errors in 4 files.
 - [ ] example-sim: @pixelzoom 20 errors in 1 files.
 - [ ] interaction-dashboard: @zepumph 558 errors in 1 files.
 - [ ] joist: @zepumph 7 errors in 1 files.
 - [ ] kite: @jonathanolson 342 errors in 3 files.
 - [ ] pendulum-lab: @jonathanolson 36 errors in 1 files.
 - [ ] ph-scale-basics: @pixelzoom 20 errors in 1 files.
 - [ ] ph-scale: @pixelzoom 20 errors in 1 files.
 - [ ] phet-core: @jonathanolson 49 errors in 3 files.
 - [ ] phet-info: @zepumph 5 errors in 1 files.
 - [ ] phet-io-sim-specific: @samreid 38 errors in 3 files.
 - [ ] phet-io-website: @chrisklus 262 errors in 12 files.
 - [ ] phet-io-wrapper-haptics: @jessegreenberg 163 errors in 1 files.
 - [ ] phet-io-wrapper-hookes-law-energy: @zepumph 9 errors in 1 files.
 - [ ] phet-io-wrapper-lab-book: @zepumph 5881 errors in 1 files.
 - [ ] phet-io-wrappers: @zepumph 154 errors in 15 files.
 - [ ] phet-lib: @jonathanolson 310 errors in 1 files.
 - [ ] phetmarks: @jonathanolson 9 errors in 1 files.
 - [ ] scenery-lab-demo:  2 errors in 1 files.
 - [ ] scenery-phet: @pixelzoom 107 errors in 3 files.
 - [ ] scenery: @jonathanolson 2955 errors in 79 files.
 - [ ] studio: @zepumph 1 errors in 1 files.
 - [ ] sun: @jbphet 2 errors in 1 files.
 - [ ] tambo: @jbphet 34 errors in 4 files.
 - [ ] wave-on-a-string: @jonathanolson 432 errors in 1 files.
 - [ ] weddell: @jbphet 16 errors in 1 files.
 - [ ] yotta: @jonathanolson 1 errors in 1 files.

[zepumph]

• Does this add too much time to our lint process? Ideally this part would be pretty well cached by lint (since html may not change too often).

[zepumph]

• I wonder if there is a way to turn off the "HTML" plugin for a specific repo or config file.

[zepumph]

Subject: [PATCH] fix lint common code a11y views, https://github.com/phetsims/chipper/issues/1408
---
Index: js/grunt/lint.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/grunt/lint.js b/js/grunt/lint.js
--- a/js/grunt/lint.js	(revision 56029937a372997654028bc63fcfb3ef2d3c77fe)
+++ b/js/grunt/lint.js	(date 1696001681767)
@@ -19,7 +19,7 @@
 const crypto = require( 'crypto' );
 
 // constants
-const EXCLUDE_REPOS = [];
+const EXCLUDE_REPOS = [ 'fenster' ];
 
 // "Pattern" is really a path, we assume here that gruntfiles help keep the right directory stucture and can just pop
 // out of the repo running the command
@@ -80,7 +80,7 @@
     // Our custom rules live here
     rulePaths: [ '../chipper/eslint/rules' ],
 
-    extensions: [ '.js', '.jsx', '.ts', '.tsx', '.mjs', '.cjs' ],
+    extensions: [ '.js', '.jsx', '.ts', '.tsx', '.mjs', '.cjs', '.html' ],
 
     // If no lintable files are found, it is not an error
     errorOnUnmatchedPattern: false
Index: eslint/.eslintrc.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/eslint/.eslintrc.js b/eslint/.eslintrc.js
--- a/eslint/.eslintrc.js	(revision 56029937a372997654028bc63fcfb3ef2d3c77fe)
+++ b/eslint/.eslintrc.js	(date 1696001652905)
@@ -16,6 +16,9 @@
   // Without a parser, .js files are linted without es6 transpilation. Use the same parser that we use for TypeScript.
   parser: '@typescript-eslint/parser',
 
+  // Lint javascript in HTML files too
+  plugins: [ 'html' ],
+
   overrides: [
     {
 
Index: eslint/.eslintignore
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/eslint/.eslintignore b/eslint/.eslintignore
--- a/eslint/.eslintignore	(revision 56029937a372997654028bc63fcfb3ef2d3c77fe)
+++ b/eslint/.eslintignore	(date 1696001652903)
@@ -5,6 +5,7 @@
 **/snapshots
 **/js/parser/svgPath.js
 **/templates/*.js
+**/templates/*.html
 **/js/**/*Strings.ts
 **/images/**/*_cur.ts
 **/images/**/*_jpg.ts
Index: package.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/package.json b/package.json
--- a/package.json	(revision 56029937a372997654028bc63fcfb3ef2d3c77fe)
+++ b/package.json	(date 1696001652907)
@@ -37,6 +37,7 @@
     "docdash": "~1.2.0",
     "eslint": "~8.28.0",
     "eslint-plugin-react": "~7.31.11",
+    "eslint-plugin-html": "~7.1.0",
     "grunt": "~1.5.3",
     "html-webpack-plugin": "~5.3.2",
     "jimp": "~0.16.1",
Index: eslint/rules/copyright.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/eslint/rules/copyright.js b/eslint/rules/copyright.js
--- a/eslint/rules/copyright.js	(revision 56029937a372997654028bc63fcfb3ef2d3c77fe)
+++ b/eslint/rules/copyright.js	(date 1696001652905)
@@ -18,6 +18,12 @@
       // Get the whole source code, not for node only.
       const comments = context.getSourceCode().getAllComments();
 
+      const isHTML = context.getFilename().endsWith( '.html' );
+
+      if ( isHTML ) {
+        return;
+      }
+
       if ( !comments || comments.length === 0 ) {
         context.report( {
           node: node,

[zepumph]

I finally got to a commit point here. I would like to touch base with @samreid about a couple of things.

1. These repos don't have any js, but do have HTML, but no package.json to describe how the html should be linted. For now I ignore them, but probably having an eslintrc at the top level would be best, right? 'fenster', 'decaf', 'scenery-lab-demo'
2. There are 78 files that are disabling lint with a TODO to this issue.
3. the above checkboxes seem worthy of a conversation.
   • Does this add too much time to our lint process? Ideally this part would be pretty well cached by lint (since html may not change too often).
   • I wonder if there is a way to turn off the "HTML" plugin for a specific repo or config file.
4. I want to look at how I needed to override 2 things, bad-sim-text and no-multiple-empty-lines. Both of these rely on *.html instead of a more general support that the plugin claims: https://github.com/BenoitZugmeyer/eslint-plugin-html#htmlhtml-extensions
5. The current strategy for "ignoring" templated files that are names .html is to put them into the eslint ignore file. Is that good?

   chipper/eslint/.eslintignore

   Lines 23 to 24 in 828c6e3

   	../phet-io-wrappers/common/html/customPhetioWrapperTemplateSkeleton.html
   	../phet-io-wrappers/common/html/standardPhetioWrapperTemplateSkeleton.html

[zepumph]

Discussed during dev meeting today:

1. Do we care about the code quality in HTML scripts as much as production code (generally)? Yes provided that it isn't too costly.
2. Should we go back and fix these eagerly? No. We want to know about these TODOs, so that next time someone is working on an HTML file with a disable directive, they should likely fix lint as part of that new work in the file. We place trust in developer discretion.
3. Lint should not be disabled as a whole on new files, instead you should support lint.
4. @samreid wanted to make sure people were aware that one-off html files could be quite easy to support TypeScript approach. To make sure that works well, update three spots:
   1. chipper/tsconfig/all/tsconfig.json
   2. top level tsconfig.json in your repo
   3. Transpiler.js make sure it supports transpiling your file (default support is

      chipper/js/common/Transpiler.js

      Lines 31 to 38 in 2356daa

      	// Directories in a sim repo that may contain things for transpilation
      	// This is used for a top-down search in the initial transpilation and for filtering relevant files in the watch process
      	const subdirs = [ 'js', 'images', 'mipmaps', 'sounds', 'shaders', 'common', 'wgsl',
      	// phet-io-sim-specific has nonstandard directory structure
      	'repos' ];
      	const getActiveRepos = () => fs.readFileSync( '../perennial-alias/data/active-repos', 'utf8' ).trim().split( '\n' ).map( sim => sim.trim() );

      ), (custom logic to add a one-off item is something like

      chipper/js/common/Transpiler.js

      Lines 315 to 330 in 2356daa

      	if ( repo === 'sherpa' ) {
      	// Our sims load this as a module rather than a preload, so we must transpile it
      	this.visitFile( Transpiler.join( '..', repo, 'lib', 'game-up-camera-1.0.0.js' ) );
      	Object.keys( webpackGlobalLibraries ).forEach( key => {
      	const libraryFilePath = webpackGlobalLibraries[ key ];
      	this.visitFile( Transpiler.join( '..', ...libraryFilePath.split( '/' ) ) );
      	} );
      	}
      	else if ( repo === 'brand' ) {
      	this.visitDirectory( Transpiler.join( '..', repo, 'phet' ) );
      	this.visitDirectory( Transpiler.join( '..', repo, 'phet-io' ) );
      	this.visitDirectory( Transpiler.join( '..', repo, 'adapted-from-phet' ) );
      	this.brands.forEach( brand => this.visitDirectory( Transpiler.join( '..', repo, brand ) ) );
      	}

      )

[zepumph]

I fixed a couple more files I felt like were important, I'm going to unassign from here, and let the TODOs point to this issue. Basically I'm fine closing this issue, or letting it last forever having eslint-disable directives point here. If anyone feels differently, please update the TODOs (remove them?) and feel free to close.

[samreid]

This feature is having problems with ESLint 9, see #1451 (comment)

[samreid]

It looks like we got JS in HTML linting again, I can test it by renaming beers-law-lab_en.html to test.html and running grunt lint.

[phet-dev]

Reopening because there is a TODO marked for this issue.

[samreid]

Oops forgot about the 60+ TODOs here, let's leave open until those are addressed.