phetsims / phet-vite-demo

Template/proof-of-concept of using Vite with PhET sources without the PhET toolchain
MIT License
1 stars 0 forks source link

ESLint Configuration file (Missing eslint.config.js) #1

Open veillette opened 1 month ago

veillette commented 1 month ago

I'm following the instructions as given on https://github.com/phetsims/phet-vite-demo/blob/main/README.md

My machine runs Windows 11 but I used WSL2 to run the various commands.

Everything runs as expected until the step #5 in linting.

After issuing the command

 npx eslint .

I get the following message


Oops! Something went wrong! :(

ESLint: 8.28.0

ESLint couldn't find a configuration file. To set up a configuration file for this project, please run:

npm init @eslint/config

ESLint looked for configuration files in /home/veillette/phet-vite-demo/dist/assets and its ancestors. If it found none, it then looked in your home directory.

If you think you already have a configuration file or if you need more help, please stop by the ESLint chat room: https://eslint.org/chat/help


The message is clear on why the previous command failed, so now I run

 npm init @eslint/config

At this point, though, the command line becomes interactive and prompts me with numerous questions that, as a novice, I am not sure how to answer in this situation.


✔ How would you like to use ESLint? · problems ✔ What type of modules does your project use? · esm ✔ Which framework does your project use? · none ✔ Does your project use TypeScript? · typescript ✔ Where does your code run? · browser The config that you've selected requires the following dependencies:

eslint, globals, @eslint/js, typescript-eslint ✔ Would you like to install them now? · No / Yes ✔ Which package manager do you want to use? · npm

added 139 packages, changed 1 package, and audited 296 packages in 4s


For a novice, it would be a better experience if this repo included an eslint.config.js file that would be appropriate.

veillette commented 1 month ago

Assigning to @jonathanolson

jonathanolson commented 1 month ago

Fails for me also, looks like it broke recently with our common work.

zepumph commented 1 month ago

Looks like we ditched the previous config and started importing our root file.

https://github.com/phetsims/phet-vite-demo/commit/c0869e19df62415a57c1e6bf88f37ee634968be6

Let's see if I can bring back the old config in the new flat format.

zepumph commented 1 month ago

I think it will be good to check in with @jonathanolson. It isn't clear to me how we want this repo being linted. The previous config didn't use any of the typescript plugins even though they are installed, and I'm curious if we can instead use shared packages to cut down on this kind of problem in the future (where "eslint" versions are hard coded in multiple package.json objects and so don't get updated correctly).

zepumph commented 1 month ago

A couple notes for myself:

```diff Subject: [PATCH] doc, https://github.com/phetsims/chipper/issues/1499 --- Index: phet-vite-demo/eslint.config.mjs IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/phet-vite-demo/eslint.config.mjs b/phet-vite-demo/eslint.config.mjs --- a/phet-vite-demo/eslint.config.mjs (revision d7f654a9c54c59a61b618e1fd7dd8b071eec0da0) +++ b/phet-vite-demo/eslint.config.mjs (date 1729536854543) @@ -3,20 +3,43 @@ /** * ESLint configuration for phet-vite-demo. * + * @author Jonathan Olson * @author Sam Reid (PhET Interactive Simulations) * @author Michael Kauzmann (PhET Interactive Simulations) */ -import browserEslintConfig from '../chipper/eslint/browser.eslint.config.mjs'; +import { globals } from '../chipper/eslint/root.eslint.config.mjs'; export default [ - ...browserEslintConfig, + { ignore: [ 'dist' ] }, { rules: { 'no-bitwise': 'off' - } - }, - { - ignores: [ 'dist' ] + }, + overrides: [ + { + files: [ + '**/*.ts' + ], + parserOptions: { + sourceType: 'module' + } + } + ], + env: { + browser: true, + es6: true + }, + languageOptions: { + parserOptions: { + globals: { + ...globals.browser, + ...globals.es2018 + }, + + ecmaVersion: 8, + sourceType: 'module' + } + } } ]; \ No newline at end of file Index: chipper/eslint/root.eslint.config.mjs IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/chipper/eslint/root.eslint.config.mjs b/chipper/eslint/root.eslint.config.mjs --- a/chipper/eslint/root.eslint.config.mjs (revision 48f62a8f9a992ff02bcc262e5b6d50018c7782c9) +++ b/chipper/eslint/root.eslint.config.mjs (date 1729536796707) @@ -1760,4 +1760,7 @@ '@typescript-eslint/no-floating-promises': 'off' } } -]; \ No newline at end of file +]; + +// To share +export const globals = globals; \ No newline at end of file Index: phet-vite-demo/package.json IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/phet-vite-demo/package.json b/phet-vite-demo/package.json --- a/phet-vite-demo/package.json (revision d7f654a9c54c59a61b618e1fd7dd8b071eec0da0) +++ b/phet-vite-demo/package.json (date 1729536969403) @@ -13,11 +13,9 @@ }, "devDependencies": { "vite": "^5.1.0", - "eslint": "~8.28.0", + "eslint": "^9.13.0", "typescript": "~5.3.3", "@typescript-eslint/parser": "~6.18.1", - "@typescript-eslint/eslint-plugin": "~6.18.1", - "@typescript-eslint/utils": "~6.18.1", "@types/jquery": "~3.5.13", "@types/lodash": "~4.14.172", "@types/p2": "~0.7.39",
zepumph commented 1 month ago

From our conversation with @samreid @jonathanolson today. Let's just revert back to eslint 8. This isn't too necessary or valuable.

zepumph commented 1 month ago

npx eslint . is now working with no problems. I will say I don't believe this is actually linting with any rules. It is up to @jonathanolson to determine what the lint config for this repo is, as we determined that it shouldn't use shared configuration.