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

Move stylelistic rules to separate package #814

Open
elwayman02 opened this issue May 1, 2023 · 9 comments
Open

Move stylelistic rules to separate package #814

elwayman02 opened this issue May 1, 2023 · 9 comments

Comments

@elwayman02
Copy link

Stylelint v15 deprecated 76 stylelistic rules in v15, opting to allow tools like prettier to manage that behavior instead: https://github.com/stylelint/stylelint/blob/main/docs/migration-guide/to-15.md

This allows us to eliminate the need for prettier-specific configs, expecting them to be used together by default (or developers can choose not to use prettier if they don't care about enforcing style). Will this project follow suit? There are 14 rules considered to overlap with prettier's behavior, as noted by the stylelint-config-prettier-scss package: https://github.com/prettier/stylelint-config-prettier-scss/blob/main/src/index.js

It would make sense to me to keep consistent with the core stylelint project's approach and follow the same strategy for these SCSS rules, deprecating them and then removing in a future release.

@XhmikosR
Copy link
Member

XhmikosR commented May 2, 2023

-1 to prettier. It's a PITA to replace the stylelistic rules for Stylelint itself and thankfully someone made https://github.com/elirasza/stylelint-stylistic and prettier isn't a panacea.

@elwayman02
Copy link
Author

-1 to prettier. It's a PITA to replace the stylelistic rules for Stylelint itself and thankfully someone made https://github.com/elirasza/stylelint-stylistic and prettier isn't a panacea.

You don't have to use prettier for this change to make sense. stylelint-stylistic exists, and so could stylelint-stylistic-scss. The key is to be consistent in our approach, otherwise it will create issues if this package deviates from what the core stylelint project is doing.

For example, prettier users today have to use this package: https://github.com/prettier/stylelint-config-prettier-scss

However, that package by default extends stylelint-config-prettier, which disables the deprecated stylistic rules from stylelint itself. In a future version of stylelint, those rules will not exist at all, which will literally break builds when people try to extend stylelint-config-prettier-scss, because it's trying to disable rules that don't exist. So, we're better off keeping the approach to stylelistic rules consistent across the board, rather than the scss community taking a different stance than stylelint does in general.

@XhmikosR
Copy link
Member

XhmikosR commented May 5, 2023

I don't agree with upstream's choice about this change anyway. Why create another package? Just for the sake of it? I don't think we need to follow a bad upstream choice.

@elwayman02
Copy link
Author

It makes sense because the lint rules serve different purposes. There are rules that catch bugs and rules that enforce stylistic preferences, and it's reasonable to separate them clearly. Having them all in the same package, especially with the vast number of rules we have for linting today, creates confusion among developers. Especially with the rise of stylistic-focused tools like (but not exclusively) prettier, there is a strong reason to have linting packages that are purely focused on catching bugs. There is little to no overhead to having 2 packages (one for pure bug linting, one for stylistic linting), so developers can more easily pick and choose what they want without having to bring all the rules in one package. It's a no-brainer improvement for everyone, whether or not they use prettier.

@SimonNodel-AI
Copy link

Thank you @elwayman02 for bringing this issue up and for providing good rationale. I would also like to add a vote for rule consistency between this library and stylelint. I do like the separation of bug catching rules and stylistic rules and having flexibility to choose which way to go. We are doing a round of dependency upgrades on our project and ran into this inconsistency.

@elwayman02 elwayman02 changed the title Deprecate stylelistic rules? Move stylelistic rules to separate package May 5, 2023
@elwayman02
Copy link
Author

I've updated the title of the issue to what I think the right solution is. I'm empathetic to the need to keep the rules available for non-prettier users, and I think it's reasonable to separate those rules into their own package under the stylelint-scss org, for projects that need them.

@json-derulo
Copy link

json-derulo commented Nov 6, 2023

Not only Stylelint deprecated their formatting rules, ESLint did the same recently. In ESLint world, very popular plugins like TypeScript ESLint also don't include formatting rules. It's becoming the de-facto standard.

@kristerkari
Copy link
Collaborator

I've updated the title of the issue to what I think the right solution is. I'm empathetic to the need to keep the rules available for non-prettier users, and I think it's reasonable to separate those rules into their own package under the stylelint-scss org, for projects that need them.

This sounds like the best way forward.

Not only Stylelint deprecated their formatting rules, ESLint did the same recently. In ESLint world, very popular plugins like TypeScript ESLint also don't include formatting rules. It's becoming the de-facto standard.

It's understandable since many of the stylistic rules would not be fully compatible with tools like Prettier.

@firefoxic
Copy link

firefoxic commented Dec 14, 2023

Probably an odd question, but... If you do decide to move the stylistic rules to a separate project, will it be in that organization?

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

No branches or pull requests

6 participants