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: Update instructions for ESLint v9 #27939

Merged
merged 4 commits into from
May 7, 2024

Conversation

gingkapls
Copy link
Contributor

@gingkapls gingkapls commented May 7, 2024

Because

The recent ESLint V9 update made breaking changes to the default configuration file format, which resulted in the old instructions becoming out-of-date and some plugins to stop working with the new default format among other changes.

This PR

  • Corrects link text from eslint to ESLint
  • Removes DigitalOcean tutorial for setting up ESLint extension as it's outdated and not needed anymore
  • Adds a lesson note about the current version of ESLint not picking up configurations for workspace sub-directories and fixing that behaviour
  • Removes instructions for making ESLint and Prettier work together, as they work out of the box now

Issue

Closes #27818

Additional Information

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project curriculum contributing guide
  • The title of this PR follows the location of change: brief description of change format, e.g. Intro to HTML and CSS lesson: Fix link text
  • The Because section summarizes the reason for this PR
  • The This PR section has a bullet point list describing the changes in this PR
  • If this PR addresses an open issue, it is linked in the Issue section
  • If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
  • If any lesson files are included in this PR, they follow the Layout Style Guide

@github-actions github-actions bot added the Content: JavaScript Involves the JavaScript course label May 7, 2024
@gingkapls
Copy link
Contributor Author

@MaoShizhong There is a linting error about a missing heading for Assignment.

I have assumed that it is fine to ignore in this case.

@MaoShizhong
Copy link
Contributor

@gingkapls You can ignore the assignment linting error for the moment.

Putting this on hold while investigating further things regarding the ESLint extension, and flat config rules, as it seems we might need to tweak some stuff due to confusion new systems.

@gingkapls
Copy link
Contributor Author

@MaoShizhong eslint-plugin-react does not work with eslint v9 yet, and since Vite pulls it in as a dependency, it too defaults to using ESLint v8 and the older .eslintrc.cjs format.

This creates a discrepancy between the instructions in the Linting lesson and the ones in Setting Up a React Environment where we suggest using eslint-config-prettier.

I think the instructions in the React lesson will have to remain the same until Vite updates to ESLint v9, but I haven't reached the React section of the curriculum myself so I cannot give any further input on the impact of the discrepancy between the two lessons.

@MaoShizhong
Copy link
Contributor

@gingkapls Thanks for looking into that. I just came across the same thing regarding some non-curriculum options, which will be resolved once the necessary plugins support v9+.

For now, I'm not concerned about the React part. The linting lesson is about introducing Linters and getting started with ESLint. In the React portion, all the necessary deps and files are generated when you run the Vite CLI command, so it does everything for you. The only difference will be the eslint config won't use the flat config format (which will be fine pre-v9). Vite will update their create CLI tool for the React template once they are able to (which will likely be when `eslint-plugin.

We actually do not need that eslint-config-prettier bit, to be totally honest. All it does is disable the following rules: https://github.com/prettier/eslint-config-prettier/blob/main/index.js none of which are activated in any of the plugins that come with the Vite template:

    'eslint:recommended',
    'plugin:react/recommended',
    'plugin:react/jsx-runtime',
    'plugin:react-hooks/recommended',

So that part can actually be removed, but the rest of the React set up lesson can remain as is.

gingkapls and others added 2 commits May 8, 2024 01:16
@gingkapls gingkapls requested a review from MaoShizhong May 7, 2024 20:12
Copy link
Contributor

@MaoShizhong MaoShizhong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think everything else should be good to go, just the one small "might as well" below. Thank you for looking into and working on this as well. Not easy when the tool's release itself wasn't the cleanest...

javascript/javascript_in_the_real_world/linting.md Outdated Show resolved Hide resolved
Co-authored-by: MaoShizhong <122839503+MaoShizhong@users.noreply.github.com>
@gingkapls
Copy link
Contributor Author

It was a fun experience, and I got to help out too. Thanks for helping and guiding me throughout all this :)

@gingkapls gingkapls requested a review from MaoShizhong May 7, 2024 21:24
Copy link
Contributor

@MaoShizhong MaoShizhong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Will keep an eye on the ESLint extension for changes to releases and any settings.

@MaoShizhong MaoShizhong merged commit 5954df9 into TheOdinProject:main May 7, 2024
2 of 3 checks passed
@gingkapls gingkapls deleted the change_linting_lesson branch May 18, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content: JavaScript Involves the JavaScript course
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linting: Tutorial to setup eslint with vscode not working anymore.
2 participants