require-statement-match problems

[zepumph]

This lint rule could use some work.

1. It errored when adding a file suffix (boo), fixed poorly in 2105fc0#diff-58eb2baf9ab123b9e849f28924bb0c9ba98af12f1eff562fc30e1e54f580911fR66
2. It doesn't convert the import to camel case before comparing (this should not need a disable (https://github.com/phetsims/binder/blob/be338a47e85fcfd38c3f53ef7e5c93c09a25d9e6/js/generate.js#L12)
3. I believe we may want to force require statements to include the js suffix, but that may come with a lot of busy work, or unforeseen problems. It would probably want to add a fix option to it at least. Separate rule/issue?

[samreid]

I've revised the rule to require a .js suffix for relative paths (note do not put a .js suffix for npm requires like const fs = require( 'fs' ); This part of the rule has support for auto fix.

It has better name comparison with camel casing.

After these changes,

• lint-everything --clean is passing
• grunt check --clean is passing
• sage run chipper/js/scripts/precommit-hook-multi.js with force completes in 1835527ms with only errors in things like locales and

PhET-iO Element missing: friction.general.view.navigationBar.preferencesButton.preferencesDialogCapsule.archetype.preferencesTabs.localizationTab
friction.general.view.navigationBar.preferencesButton.preferencesDialogCapsule.archetype.selectedTabProperty._data.initialState differs. 
Expected:
{"units":null,"validValues":["OVERVIEW","VISUAL","AUDIO","LOCALIZATION"]}
 actual:
{"validValues":["OVERVIEW","VISUAL","AUDIO"],"units":null}

I also tested grunt --brands=phet in a simulation and it built correctly. If there is trouble here, it will likely be due to some entry point not dealing with the *.js suffix properly, but hopefully that won't happen.

I'll push what I have now, then we can search for usages of the disable-line to see where we may want to rename more things.

I'm also OK to separate this into another rule if that would be clearer for users or more maintainable. Right now the one rule is checking multiple aspects.

[samreid]

Changes mostly pushed. Topics to review with @zepumph

• Should we separate this into different rules?
• Review occurrences of eslint-disable directives to see if we want to rename vars, or add more support in the rule, or keep the directive.
• See if rosetta wants to turn the rule back on.

[zepumph]

I'm getting blocked on this because this was incorrectly fixed:

require( '../../../perennial/node_modules/playwright.js' )

Changing now.

[samreid]

Uh oh, I did not correctly address the case of importing an npm module from a relative path. Thanks for catching it and sorry for the trouble. It looks like you got it covered, but let me know if there more to do here.

[zepumph]

• It seems really important that we have a way to assert the import statements get a js suffix too. Separate issue?
• I cannot figure out this. When I apply this patch, linting alpenglow does not show a lint error. Can you help?

Subject: [PATCH] add authors, https://github.com/phetsims/chipper/issues/1502
---
Index: doc/generate-bibliography.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/doc/generate-bibliography.js b/doc/generate-bibliography.js
--- a/doc/generate-bibliography.js	(revision b34ba1617e253f15588841bdd1b285bc10b21dd4)
+++ b/doc/generate-bibliography.js	(date 1730747247217)
@@ -5,12 +5,13 @@
 // Generates the bibliography for the Alpenglow documentation
 // Run with `node js/generate-bibliography.js`
 
-const Cite = require( 'citation-js' ); // eslint-disable-line phet/require-statement-match
+const x;
+const citationJs = require( 'cifdsatation-js' );
 const fs = require( 'fs' );
 
 ( async () => {
   const links = {};
-  const cite = new Cite();
+  const cite = new citationJs();
 
   await cite.add( `@inproceedings{Soerjadi1968OnTC,
     title={On the Computation of the Moments of a Polygon, with some Applications},

I'm going to stop my review now here just in case it's a problem on my end we need to fix.

[samreid]

doc/ is ignored in root.eslint.config.mjs. After this patch, the error above is correctly identified:

Subject: [PATCH] add authors, https://github.com/phetsims/chipper/issues/1502
---
Index: js/eslint/root.eslint.config.mjs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/eslint/root.eslint.config.mjs b/js/eslint/root.eslint.config.mjs
--- a/js/eslint/root.eslint.config.mjs	(revision 1f5dc79e3cb523e462cc9b55402c1f87cf0ee900)
+++ b/js/eslint/root.eslint.config.mjs	(date 1730867056960)
@@ -33,7 +33,6 @@
       'templates/',
       'js/*Strings.ts',
       'images/',
-      'doc/',
       'sounds/',
       'mipmaps/',
       'assets/',

Our doc/ directories often have *.md files. alpenglow is an outlier to have so much *.js there. I wouldn't be surprised to see some *.html files in doc/ (and we do have a plugin to lint js appearing in html files). How should we proceed?

[zepumph]

Excellent! I removed that ignore from root for local testing and only saw trouble in alpenglow, so a repo-specific change seemed preferable.

It seems really important that we have a way to assert the import statements get a js suffix too. Separate issue?

Let's pick that up in phetsims/perennial#407

Ready to close.

[zepumph]

I believe we need to revert the part requiring a .js extension. In fact, we should likely assert the opposite. This is because when converting a file to ts, require() in js isn't smart enough to try to use the .ts file.

[zepumph]

I believe this is enough to unblock phetsims/perennial#403. I will report back if more is needed here.

[zepumph]

Although it is fine that this occurred, we will want to proceed with phetsims/perennial#410 instead. Re-closing and likely to revert much of the above as we convert require statements to imports.

[zepumph]

I updated the code style for Webstorm to always import require statements with the .js suffix. We should change that since this revert.