Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linting: Tutorial to setup eslint with vscode not working anymore. #27818

Closed
2 of 3 tasks
QuicksVolex1 opened this issue Apr 19, 2024 · 8 comments · Fixed by #27939
Closed
2 of 3 tasks

Linting: Tutorial to setup eslint with vscode not working anymore. #27818

QuicksVolex1 opened this issue Apr 19, 2024 · 8 comments · Fixed by #27939
Assignees

Comments

@QuicksVolex1
Copy link

Checks

Describe your suggestion

The tutorial for setting up eslint for vscode doesn't work, it's because eslint uses a new configuration file as default, but the extension expects the old one, so the fix for this is to switch to "Pre-release version" on extension, which took me a long time to figure out.

I also think that the setup used in the tutorial is not done in the same way as in the getting started section in the eslint website, but i'm not sure if this is a significant issue.

Path

Node / JS

Lesson Url

https://www.theodinproject.com/lessons/node-path-javascript-linting

(Optional) Discord Name

No response

(Optional) Additional Comments

No response

@MaoShizhong
Copy link
Contributor

Thanks @QuicksVolex1
It looks like ESLint v9 officially released 2 weeks ago and introduce a lot of changes, including some that affect the setup process - even their current docs are inaccurate, but I imagine this will be addressed in due course. It also means it might not be an easy drop-in replacement for the guide, as existing guides will still be relevant for pre-v9.

I'll bring this to the team's attention and we'll have a think/keep an eye out for the best way forward.

@Baguirre03
Copy link
Contributor

Ah. So this is why my ESLINT wasnt working today..

@gingkapls
Copy link
Contributor

gingkapls commented Apr 23, 2024

After trying out various things, I stumbled upon a method that doesn't deviate much from the guide we use in the linting lesson and still is able to get us a working .eslintrc.json using eslint v8.

  1. Create package.json using npm init -y
  2. Run npm init @eslint/config@v0.4.6
  3. Follow the rest of the steps of the digital ocean guide as it is from Step 2 prompt onwards.

I feel like it could work as a stopgap until issues with v9 and flat config are resolved.

@MaoShizhong
Copy link
Contributor

MaoShizhong commented Apr 24, 2024

@TheOdinProject/javascript Opinions on how best to handle this currently?

Summary:

  • ESLint v9+ released recently, changing a lot of things, inc. what config files are valid
  • ESLint documentation itself has inaccuracies/omits some key info, e.g. inconsistencies with what commands are referenced in different parts, saying that the @eslint/config generated file will contain stuff that it doesn't actually contain (it would be better if they provided an example of what it'd actually contain and not an example that assumes you already know about linters and how rules are configured), using ESM syntax in .js files without mentioning ESM/CJS nuances etc.
  • ESLint VSCode extension requires the pre-release version to recognise rules from the v9+ config file
  • Some external style guides e.g. airbnb-base have not yet been updated to be compatible with ESLint v9, and so sometimes incorrectly lint e.g. #foo; flags as "unexpected token #" even if used correctly in as a private class field. This will hopefully be fixed in due course when style guides start to support v9.

I did briefly look at the manual set up section, which would prevent confusion with what the config file contains, but it uses ESM syntax and the instructions only indicate .js. Learners in the Webpack section will run into issues because the webpack config will be using CJS by default, and we'd need to tell learners to touch eslint.config.mjs instead in our lesson.
So not a clean solution either.

(Non-exhaustive) options that come to mind:

  1. Remove link to ESLint's Quick Start bit, and instead provide adjusted instructions for setting up the older but currently working (and compatible with current style guides) version. i.e. Temporarily remove bullet 1. from the ESlint set up bit in our lesson. Just use the second bullet for the ESLint extension, provide the DO tutorial (which includes setting up the dependency and config file), and just add our own notes so learners run the @eslint/config version that's compatible with the tutorial. When ESlint v9 is fully stable with existing style guides and documentation is actually fixed, update our lesson with the latest instructions.
  2. Find some way to adjust the current lesson instructions to account for ESLint v9+, remove resources for pre-v9 things, and just wait for style guides like airbnb-base to get their act together and support v9+.

@MaoShizhong
Copy link
Contributor

MaoShizhong commented May 3, 2024

I've been following the repos for ESLint and their create-config CLI tool (the one given in ESLint's current "Getting Started" guide). Fortunately, a lot of things have been updated and are less confusing, and actually resemble what happens.

  1. The current "Getting Started" guide that uses npm init @eslint/config@latest now no longer forces a style guide selection on the user.
    image
  2. The generated eslint.config.mjs file contains almost only the contents displayed in the "getting started" guide.
    import globals from "globals";
    import pluginJs from "@eslint/js";
    
    export default [
      {languageOptions: { globals: globals.browser }},
      pluginJs.configs.recommended,
    ];
  3. Some popular external plugins such as eslint-config-airbnb-base are yet to support ESLint v9. I would imagine they will eventually. I'm not sure how much of a consideration this might be for us for now. On the one hand, it's a very popular plugin. On the other hand, it's no longer enforced or selectable by the create-config CLI tool, and I'm not sure if a single plugin's slow reaction speed should be a blocker for us in terms of just providing updated ESLint setup instructions.
  4. The ESLint VSCode extension's release version (2.4.4) now supports ESLint v9's flat config, so we no longer need any special instruction to switch to pre-release.
  5. eslint-config-prettier should no longer be required, nor any special instructions for using Prettier with ESLint v9+, as the ESLint:recommended ruleset that comes with the create-config command no longer includes any stylistic rules outright - no longer any clashes with Prettier. This can be confirmed by going to the ESLint rules list and checking all rules that have a filled green ☑️. For easier checking, you can filter out all the rules not in the recommended ruleset by going to the browser console and running:
    document.querySelectorAll('article.rule:has(p:first-of-type[aria-hidden="true"])').forEach((article) => article.style.display = "none")

Therefore, assuming step 3 is not a blocker for our lesson purposes, we could amend the Linting lesson's Linting section to only mention the ESLint "Getting Started" page, and link to the VSCode extension. I don't think we need a tutorial for using the extension and the package together since there's nothing special needed to "link" them - just install both separately. Perhaps just mention it's wise to have both so you get automatic linting highlights in VSCode but it'll also pick up a project's eslint.config.mjs file if present.

We can then just get rid of the ESLint + Prettier part altogether, or keep only the "you might want to use both". None of the conflict stuff will be relevant for ESLint v9+.

@gingkapls
Copy link
Contributor

Seems like the lesson guide could be even more streamlined than before

I agree that airbnb-base shouldn't be a blocker. The init wizard removed the options for style-guides and it doesn't seem that it'll ever be added back again, since they suggest using the --config option instead.

When airbnb-base does add support for v9 and we decide to include it, we would still have to write up additional instructions since it wouldn't be automatically added through the init-wizard.

So the instructions for setting up the basic eslint config would remain the same regardless, and there would only be specific parts related to --config that would need to be added in the future (if we do decide to support airbnb-base)

@MaoShizhong
Copy link
Contributor

We should be good to go with the changes I've highlighted above:

  1. Remove the DO tutorial for the extension in the Linting section of the Linting lesson.
    • We should probably also add a temporary lesson-note box just saying that the current version of the extension (2.4.4) will only pick up the workspace folder's eslint config file, and not a config file for a subdirectory of that workspace. Switching to the pre-release version solves this. We can remove this note box once they bump the extension release version to accept these fixes.
  2. In the Linting section, there's a link with the text eslint which should be fixed to ESLint.
  3. In the Using ESLint and Prettier section, remove everything starting from "However, using ESLint and Prettier together causes conflicts...", as that will no longer be relevant for ESLint v9 and the default recommended rule set.

@gingkapls since you've been particularly active with looking into this as well, and the issue author has not checked the assignment box, would you like to be assigned to make these changes?

@gingkapls
Copy link
Contributor

Sure! I'd be happy to work on it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants