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

fix(stylelint) : Remove stylelint-processor-styled-components and stylelint-config-styled-components #55

Merged
merged 1 commit into from
Dec 9, 2020

Conversation

meriouma
Copy link
Contributor

@meriouma meriouma commented Nov 17, 2020

Motivation : styled-components/stylelint-processor-styled-components#278
La syntaxe css-in-js est maintenant majoritairement supportée par stylelint. Il y a encore quelques petits problèmes, mais ils travaillent là dessus. stylelint/stylelint#4574

@meriouma meriouma requested a review from a team November 17, 2020 16:43
Comment on lines 7 to 10
"no-empty-source": null,
"no-missing-end-of-source-newline": null,
"property-no-vendor-prefix": true,
"value-no-vendor-prefix": true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

J'ai ramené les règles de stylelint-config-styled-components, ça a pas changé depuis 4 ans, alors je voyais plus la pertinence d'avoir une dep pour ça

@@ -9,12 +9,11 @@
}
],
"font-weight-notation": "numeric",
"indentation": 4,
"indentation": 2,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

C'est pour matcher l'indent dans le .editorconfig

"order/order": [
"custom-properties",
"declarations"
],
"order/properties-alphabetical-order": true
},
"syntax": "scss"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

J'ai enlevé syntax, la doc de stylelint dit que ça détecte la syntaxe selon le type de fichier

"rules": { },
"syntax": "scss"
"rules": {
"indentation": 4,
Copy link
Contributor

Choose a reason for hiding this comment

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

Ça serait pas plutôt 2 pour matcher l'autre config? Ou IJ utilise l'indentation du JS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

En fait IJ est complètement mêlé dans le css-in-js, il reformate rien dans les tagged template literals. On avait mis 4 pour que ça match l'indent du code. Mais sinon on peut mettre 4 au scss et le changer dans le .editorconfig aussi. Tu préfères quoi?

Copy link
Contributor

Choose a reason for hiding this comment

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

Je mettrais 4 au scss sinon risque d'avoir des problèmes si on fait des fichiers scss dans un projet qui a styled-components.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

D'ac. Je viens de voir que tu passes stylelint avec la même config pour les fichiers scss et ts/tsx. Dans dabench on avait une config pour chaque, mais c'est sûrement plus simple d'en avoir juste une. Rendu là, je sais pas pourquoi on a 2 packages? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Ben c'est un peu ce que je me demandais :p

Copy link
Contributor Author

Choose a reason for hiding this comment

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

J'ai flushé le package et mergé en stylelint-config finalement

@meriouma meriouma force-pushed the dev/remove-processor branch 3 times, most recently from e811772 to 8a3d485 Compare November 20, 2020 18:43
@meriouma meriouma merged commit 09aa6fe into master Dec 9, 2020
@meriouma meriouma deleted the dev/remove-processor branch December 9, 2020 18:00
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 this pull request may close these issues.

2 participants