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

Setting Up A React Environment: Remove mention of eslint-config-prettier #28138

Closed
3 tasks done
T0nci opened this issue Jun 7, 2024 · 7 comments · Fixed by #28306
Closed
3 tasks done

Setting Up A React Environment: Remove mention of eslint-config-prettier #28138

T0nci opened this issue Jun 7, 2024 · 7 comments · Fixed by #28306
Assignees

Comments

@T0nci
Copy link
Contributor

T0nci commented Jun 7, 2024

Checks

Describe your suggestion

Since eslint-config-prettier isn't mentioned in the Linting lesson in the JavaScript course, the mention of eslint-config-prettier should also be removed from this lesson. I opted to opening an issue rather than Improve on GitHub since linting errors couldn't be fixed through GitHub if there are any.

The mention is located in this section: https://www.theodinproject.com/lessons/node-path-react-new-setting-up-a-react-environment#keeping-it-clean

Path

Node / JS

Lesson Url

https://www.theodinproject.com/lessons/node-path-react-new-setting-up-a-react-environment

(Optional) Discord Name

No response

(Optional) Additional Comments

No response

@MaoShizhong
Copy link
Contributor

I asked for that bit to be removed in #27939 but it wasn't and somehow I didn't notice and forgot all about it...
Thanks for picking it up @T0nci!

@T0nci
Copy link
Contributor Author

T0nci commented Jun 10, 2024

@MaoShizhong When looking into this issue, I saw that the Vite template does not use ESLint v9, so I thought I should ask about your opinion on this.

Since some learners might still be using Prettier alongside ESLint, they won't have a problem using them in vanilla JavaScript. But the problem arises when learners want to use Prettier with ESLint v8 and below, when there might be conflicting rules between Prettier and ESLint.

Wouldn't it be better to include the mention of eslint-config-prettier as a new package that's recommended because problems might arise when using Prettier with the ESLint version Vite uses, and then when Vite updates their template to use ESLint v9 we should change this section again?

The alternative being remove the whole Keeping it clean section so we don't have to worry when Vite will update their template to use ESLint v9 if at all.

@MaoShizhong
Copy link
Contributor

@T0nci it still won't be an issue for v8 and below, since the base ruleset still doesn't include the deprecated style rules.

Having said that, I'm exploring having the Linting lesson's bit tweaked to use the v8.57 docs and a note about v8.57 and v9+ compatibility. Plugins like airbnb and airbnb-base are not yet compatible with v9 and look to be blocked for a while yet. They also actually turn on some style rules that plugin-prettier will need to turn off. So it's not to do with eslint anymore but technically some other plugins.

You'll have ti give me a bit for this since I'm down and out with Covid

@T0nci
Copy link
Contributor Author

T0nci commented Jun 16, 2024

@MaoShizhong Please, take your time to rest up! This issue won't go anywhere, and like you always say we're all volunteers. Hope you feel better soon!

@MaoShizhong
Copy link
Contributor

MaoShizhong commented Jun 21, 2024

@T0nci I've been thinking about this some more and I think I still want the eslint-config-prettier part removed from the respective React lesson.

eslint-config-prettier will only be relevant if someone goes out of their way to use a community plugin for ESLint that includes style rules that are duplicated in Prettier, for example airbnb. Most community plugins, as well as the ESLint default recommended and the Vite React template's default plugins, do not contain any of these rules, as they are deprecated and were moved to the Stylistic linter package instead.

Therefore for most people, eslint-config-prettier is redundant. A React setup lesson feels like a weird place to put a reminder/tip about an ESLint plugin that's specifically only useful when using certain ESLint community plugins, i.e. not ESLint by default.

I have some specific changes I want to update the Linting lesson with in regards to flat config/v9 support, as well as eslint-config-prettier - I'll PR these in due course. In the meantime, could you continue with PRing the removal from the React lesson?

@T0nci
Copy link
Contributor Author

T0nci commented Jun 29, 2024

@MaoShizhong Yes, of course!

About the removal, should I remove the whole section or mention Prettier in place of eslint-config-prettier to remind the user of Prettier if they haven't been using it up to this point?

@MaoShizhong
Copy link
Contributor

MaoShizhong commented Jun 29, 2024

@MaoShizhong Yes, of course!

About the removal, should I remove the whole section or mention Prettier in place of eslint-config-prettier to remind the user of Prettier if they haven't been using it up to this point?

I'd just remove the entire "keeping it clean" section. It's clear it ships with eslint. If people have or haven't been using prettier so far, they'll continue as such. Seems a bit odd to mention it there, especially when eslint-config-prettier is technically only relevant for specific scenarios (i.e. not relevant for the default setup). Would definitely be strange to go into the technicalities there (that's all covered in the Linting lesson with recent changes now)

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.

2 participants