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

Help us prepare for the next major release of stylelint #541

Closed
jeddy3 opened this issue Sep 17, 2021 · 16 comments
Closed

Help us prepare for the next major release of stylelint #541

jeddy3 opened this issue Sep 17, 2021 · 16 comments

Comments

@jeddy3
Copy link
Collaborator

jeddy3 commented Sep 17, 2021

We've been working on a new major release of stylelint. There are quite a few changes (and a lot of refactoring). One of the biggest changes we've made is removing the syntax option and no longer bundling syntaxes in stylelint itself. Users will need to extend stylelint (via shared-configs, plugins and custom syntaxes) to lint something other than vanilla CSS.

To make this easier for users to do, we've added two properties to the configuration object:

  • customSyntax - which accepts an PostCSS syntax like postcss-scss
  • overrides - which allows users to configure stylelint to lint two or more languages, e.g. CSS and SCSS

We intend to improve the migration experience by updating our docs and by adding helpful error messages to stylelint when a user, for example, tries to lint an *.scss file but hasn't specified a custom syntax in their configuration object.

This last part is why we're reaching out to you. We want to encourage users to extend a shared-config for the language they are linting, rather than add custom syntaxes and plugins themselves. We think that'll be easier for them.

We also think emphasising the standard configs over the recommended ones will help users, especially new ones. Including those who use stylelint and a pretty-printer like Prettier together, as there are a number of stylistic rules in stylelint and this plugin that are complementary to Prettier, e.g. the *-empty-line-before ones.

To help us do this, will you (or any of the other contributors to this package) be willing to?:

  1. Update stylelint-config-recommended-scss to bundle postcss-scss
  2. Create and publish stylelint-config-standard-scss that would:
// stylelint-config-recommended-scss
module.exports = {
  extends: ["stylelint-config-recommended"],
  plugins: ["stylelint-scss"],
  customSyntax: "postcss-scss",
  rules: {
    "at-rule-no-unknown": null,
    "scss/at-rule-no-unknown": true,
    "no-invalid-position-at-import-rule": null
  }
};
// stylelint-config-standard-scss
module.exports = {
  extends: [
    "stylelint-config-standard",
    "stylelint-config-recommended-scss"
  ],
  rules: {
    "block-closing-brace-newline-after": ["always", { "ignoreAtRules": ["else"] } ],
    "scss/at-else-closing-brace-newline-after": "always-last-in-chain"
    ..
  }
};

We'll then update our Getting started guide and add errors messages to stylelint to encourage users to install stylelint-config-standard-scss when linting SCSS.

We've seen users linting SCSS with stylelint who are unaware of this plugin and all the amazing rules within it. We hope these changes will allow more users to benefit from this plugin.

Lastly, many thanks to you (and the other contributors) for maintaining this package! It's been fantastic to see the number of rules grow over the years.

@kristerkari
Copy link
Collaborator

Thanks @jeddy3! Looks like there are quite a few things that need to get done.

The syntax option change sounds like a big breaking change, so probably the SCSS related packages need a major version bump.

@niksy
Copy link
Member

niksy commented Sep 20, 2021

The syntax option change sounds like a big breaking change, so probably the SCSS related packages need a major version bump.

I think it definitely should be major version bump since PostCSS 8 is used internally in new Stylelint version.

@kristerkari what can we do to help with this?

@jeddy3
Copy link
Collaborator Author

jeddy3 commented Sep 25, 2021

The syntax option change sounds like a big breaking change

Yes, it's not a decision we took lightly. It mainly affects users, not plugin authors. We hope to make the migration for users as smooth as possible by suggesting they extend shared configs that bundle the appropriate syntax for them.

Looks like there are quite a few things that need to get done.

I believe there are two user-facing things the SCSS community need to do:

  • add the customSyntax property to stylelint-config-recommended-scss
  • create stylelint-config-standard-scss

so probably the SCSS related packages need a major version bump.

This plugin shouldn't need any user-facing changes and should already be compatible with the upcoming release of stylelint. We migrated to PostCSS 8 internally, but plugins are made compatible by stylelint itself.

Adding the customSyntax to stylelint-config-recommended-scss is also non-breaking as the current version of stylelint will silently ignore the property.

@kristerkari what can we do to help with this?

There are quite a few moving parts. It would be good to get some pull requests open so that we can see the parts working together. I suggest:

  • create pull request to add customSyntax to stylelint-config-recommended-scss
  • create a stylelint-config-standard-scss repo and add a pull request to:
  • create a pull request in this repo to update the stylelint dev dependency

All pull requests can use the v14 branch of stylelint for now.

We'd then be able to test the upcoming stylelint-config-recommended-scss package, which under the hood will test:

  • the upcoming release of stylelint (with its customSyntax option)
  • the upcoming release of stylelint-config-standard

Updating the stylelint dev dependency in this package will require tweaking the tests. You can use the stylelint jest preset (see #470) and then replace syntax: "scss" with customSyntax: "postcss-scss" in your tests files. However, you work around the issue more easily by requiring postcss-scss in your jest setup script by updating this line to customSyntax: require("postcss-scss").

Feel free to at-mention me in these pull requests, and I'll be happy to take a look.

@kristerkari Do those suggestions sound good to you? It looks like @niksy would be happy to get the ball rolling.

@jeddy3
Copy link
Collaborator Author

jeddy3 commented Sep 27, 2021

@niksy Fantastic work on #543 & stylelint-scss/stylelint-config-recommended-scss#83. I'm going offline for a week now, but I'm happy to continue helping out when I'm back. In the meantime, if you like you can get the ball rolling on creating a stylelint-config-standard-scss repo as I'm sure there'll be some discussion around:

  • how the stylelint-config-standard rules should be configured for SCSS
  • which rules in this plugin should be turned on and how they should be configured

For context, stylelint-config-standard enforces:

  • the most common conventions found in the examples of the specifications
  • common patterns found in a handful of popular CSS styleguides

Perhaps, something similar exists for SCSS, e.g. examples in the SCSS Handbook and popular SCSS styleguides?

In your pull request, I suggest using the branches of these pull requests as your dependencies:

i.e.:

  • github:niksy/stylelint-config-recommended-scss/#stylelint-v14
  • github:stylelint/stylelint-config-recommended#add-rules

And the v14 branch of stylelint. You'll then be working with the latest of everything.

@niksy
Copy link
Member

niksy commented Sep 28, 2021

@jeddy3

I'm going offline for a week now, but I'm happy to continue helping out when I'm back.

Happy vacationing! :)

In the meantime, if you like you can get the ball rolling on creating a stylelint-config-standard-scss repo as I'm sure there'll be some discussion around

@kristerkari can you create that repo so we can get started? Sass related stuff is under your username so it’s best to keep it there. Or maybe we can create stylelint-scss GitHub organization?

@kristerkari
Copy link
Collaborator

@kristerkari can you create that repo so we can get started? Sass related stuff is under your username so it’s best to keep it there. Or maybe we can create stylelint-scss GitHub organization?

I'm not sure if it makes sense to add more stuff under my name, it would prefer to start adding stuff under stylelint org or then create a new stylelint-scss org.

@jeddy3
Copy link
Collaborator Author

jeddy3 commented Oct 6, 2021

A stylelint-scss org sounds good to me as the stylelint org is limited to standard CSS syntax.

@niksy I suggest going ahead, creating it and starting to work on the standard config.

@niksy
Copy link
Member

niksy commented Oct 11, 2021

@jeddy3 I guess we can use Sass Guidelines and Sass Style Guide as starting point.

Is having standard config blocker for release?

@kristerkari is it okay for me to open organization or shall I leave that to you and then you can add me as contributor?

@jeddy3
Copy link
Collaborator Author

jeddy3 commented Oct 11, 2021

@jeddy3 I guess we can use Sass Guidelines and Sass Style Guide as starting point.

Yes, that'll do.

Is having standard config blocker for release?

No, but it would be fantastic if it were ready for release. We think per language shared-config's will help reduce user confusion and plug gaps in the linter's coverage. We'd like to start advocating for their use in the 14.0.0 migration guide.

We like to give the SCSS community a little more time. However, we'll probably need to release in a week regardless as it's been a long road getting to 14.0.0 to the finish line!

I suggest opening the organisation yourself as you can invite others as equal owners at any point.

@jeddy3
Copy link
Collaborator Author

jeddy3 commented Oct 11, 2021

I'm around this week and can help out. Just at-mention me in any repos.

@niksy
Copy link
Member

niksy commented Oct 12, 2021

@jeddy3 @kristerkari I went ahead and created organization (I’ve sent you invites) and published repository for standard config. It’s based on recommended SCSS config: https://github.com/stylelint-scss/stylelint-config-standard-scss

If everything looks OK, I will create PR for updates for v14.

@kristerkari I will leave to you to switch your repositories to organization

@jeddy3
Copy link
Collaborator Author

jeddy3 commented Oct 12, 2021

Excellent, thank you @niksy.

I've just noticed a stylelint-config-standard-scss package is already published on the registry

@moeriki Thank you for creating and publishing stylelint-config-standard-scss. Would you be willing to share ownership of it with us under the SCSS Stylelint organisation? We're preparing for a major release of Stylelint and being able to prepare and publish a new version of stylelint-config-standard-scss for it would be fantastic.

@moeriki
Copy link

moeriki commented Oct 12, 2021

I have little memory of this 😀 I can share things.

Never done this so not sure. I see I can invite a maintainer on the npm package. Is that what you would need?

The repo itself I think I'll archive with a link at the top of the README to a repo of yours.

@jeddy3
Copy link
Collaborator Author

jeddy3 commented Oct 12, 2021

I can share things.

@moeriki Fantastic, thank you!

Never done this so not sure. I see I can invite a maintainer on the npm package. Is that what you would need?

Yes, please. If you can add me ("jeddy3" on npm) as a maintainer on NPM, then I can grant access to @niksy and @kristerkari.

The repo itself I think I'll archive with a link at the top of the README to a repo of yours.

That works.

Many thanks again! There are quite a few moving pieces involved in the next release of stylelint and this will help us out a lot.

@kristerkari
Copy link
Collaborator

Thanks a lot for taking care of the stuff,

I'll have a look tonight at switching the orgs etc.

@jeddy3
Copy link
Collaborator Author

jeddy3 commented Oct 19, 2021

Closing as pull requests prepared and reviewed ready for release.

Thanks everyone!

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

4 participants