Update stylelint to v13.0.0 #428

XhmikosR · 2020-01-12T12:54:35Z

Requires #427 merged and tests to be fixed

Closes #433

XhmikosR · 2020-01-12T13:10:01Z

package.json

  },
  "peerDependencies": {
-    "stylelint": "^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 || ^12.0.0"
+    "stylelint": "^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 || ^12.0.0 || ^13.0.0"


BTW this is getting too long. We should probably drop some versions from here.

XhmikosR · 2020-01-12T14:56:21Z

@hudochenkov: maybe you could have a quick look? It seems all of the percent-placeholder-pattern tests fail with v13.0.0. After some quick debugging it seems the cause it's stylelint/stylelint#4483

hudochenkov · 2020-01-12T16:34:08Z

Your discovery is correct. This plugin uses internals of stylelint, which are not a public API.

https://github.com/kristerkari/stylelint-scss/blob/26097c6fd72aab40d76a90f176097ccd57a6c347/src/rules/percent-placeholder-pattern/index.js#L44-L52

https://github.com/kristerkari/stylelint-scss/blob/26097c6fd72aab40d76a90f176097ccd57a6c347/src/utils/isStandardSelector.js

https://github.com/kristerkari/stylelint-scss/blob/26097c6fd72aab40d76a90f176097ccd57a6c347/src/utils/isStandardRule.js

I would suggest copy required utils from stylelint (isStandardSelector, isStandardRule) prior change we made in stylelint/stylelint#4483.

stylelint concerns about standard CSS only, that's why we have these utils. stylelint-scss expand concern by allowing Sass syntax. That's why utils should be different for both projects.

XhmikosR · 2020-01-12T16:39:06Z

Thanks for the reply :) TBH I think stylelint-scss should not duplicate everything, hence why I made #430, because I see a lot seemingly identical code.

That being said, for the time being, I will try first your suggestion so that we can update to the latest version and leave the other refactoring changes for later :)

hudochenkov · 2020-01-12T20:46:09Z

TBH I think stylelint-scss should not duplicate everything, hence why I made #430, because I see a lot seemingly identical code.

This comes with a price, that something could break even with patch release of stylelint :)

I think you need to change isStandardRule only for now.

XhmikosR · 2020-01-12T21:50:32Z

I was under the impression that utils were part of the public API too. Is there a list of things that are officially part of the public API?

hudochenkov · 2020-01-12T21:54:28Z

https://stylelint.io/developer-guide/plugins#stylelintutils

XhmikosR · 2020-01-12T21:56:48Z

The thing is that stylelint.utils is pretty vague IMHO. I'd need to print the object to see this. Would be nice if the docs covered this somehow.

Or does it mean that only the mentioned utils methods are exposed?

hudochenkov · 2020-01-12T22:02:07Z

I think only exposed methods are available.

XhmikosR · 2020-01-12T22:05:22Z

Hmm, then I guess we need the opposite of #430 so that we are safe. Have you guys thought about exposing the other utils too? Currently there's like 500 lines of code and 15 files that are duplicated, and if we add more files this will keep increasing.

hudochenkov · 2020-01-12T22:07:23Z

There were never discussion about this. You can start one in stylelint repo :)

github-actions · 2020-01-13T15:58:56Z

Coverage decreased (-0.2%) to 92.06% when pulling 080922f on stylelint-13 into 26097c6 on master.

XhmikosR · 2020-01-13T16:17:27Z

@kristerkari I think this is ready for review after #427 is merged. I added the upstream code and tests before the breaking change.

kristerkari · 2020-01-13T20:49:13Z

Thanks! I've been away from home for the weekend and at work today the whole day, but I'll try to get this reviewed asap.

kristerkari · 2020-01-13T21:25:55Z

btw. I merged #427

XhmikosR · 2020-01-14T06:05:14Z

Rebased. I'm not sure if the new upstream files I backported make all sense; I see code for Less etc.

XhmikosR · 2020-01-14T16:05:27Z

I split this for easier review.

XhmikosR · 2020-01-19T22:51:42Z

@kristerkari frinedly ping for this and the other open PRs

kristerkari · 2020-01-22T21:33:02Z

package.json

  },
  "peerDependencies": {
-    "stylelint": "^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 || ^12.0.0"


Hmm... I'm still not sure if it's a good idea to drop support for everything else than the newest version.

Well, the thing is that

what was the last time you tested stylelist-scss master with stylelint 8.0.0? Or any version other than the latest?

if you are going to make sure every future version works with stylelint, why don't you just use >=?

This should be a major release IMO and we should try to merge Remove babel. #429 too, which makes it a good time to remove this

I can revert it BTW, my initial patch didn't change this.

what was the last time you tested stylelist-scss master with stylelint 8.0.0? Or any version other than the latest?

I don't think that you really understand my point of view here. I don't really care about stylelint version 8, and I don't know if it has even any users other than some outdated projects. I just don't think that it is a good idea to drop the support for 5 versions at the same time, as the chances that there are people using some those versions is really high.

I actually have one React Native project where I had to lock the stylelint version to v9.x because newer stylelint versions depend on some dependency that uses @types/node which messes up the Typescript typings for React Native in my project and causes Typescript to throw errors.

I've ran into similar issues with other npm packages, where I could not update to the latest version of a package because it required the newest version of a peer dependency, so the only solution is not to update.

The thing is that managing peer dependency versions is a complicated thing, and IMO the best way is to gradually drop supported versions from the end and release those in separate releases.

if you are going to make sure every future version works with stylelint, why don't you just use >=?

Because I don't know if the version that we have now works with a stylelint version that gets released two years from now. That's why the versions are whitelisted.

Hence why I reverted that change and I just added v13.0.0 in peerDependencies.

I still believe this should be cleaned up later.

Yes, it definitely should be cleaned up, but I need to see if there is any data about downloads per stylelint version to get an idea if people are still using those old versions.

Not sure where you can find such metrics TBH.

Anyway, I assume this PR should be good to go as is? I need to rebase a few of my other PRs after this gets merged.

pustovalov · 2020-01-23T22:59:44Z

WDYT about drop node 8 support?
stylelint/stylelint#4500

XhmikosR · 2020-01-24T09:02:42Z

The thing is that it seems stylelint-scss works with Node.js 8.x and maybe old stylelint versions.

IMHO we should grab this opportunity and:

drop Node.js < 10 support
drop babel (see Remove babel. #429)
reduce the supported stylelint versions in peerDependencies since we only test the latest

XhmikosR · 2020-01-27T11:24:43Z

Friendly ping @kristerkari ^^

XhmikosR commented Jan 12, 2020

View reviewed changes

XhmikosR force-pushed the stylelint-13 branch 2 times, most recently from 8f1bac9 to 8683ca7 Compare January 13, 2020 16:17

XhmikosR force-pushed the stylelint-13 branch from 8683ca7 to 2b0d3da Compare January 13, 2020 16:27

ntwb mentioned this pull request Jan 14, 2020

Stylelint v13 support #433

Closed

XhmikosR force-pushed the stylelint-13 branch from 2b0d3da to c22219e Compare January 14, 2020 05:57

XhmikosR marked this pull request as ready for review January 14, 2020 06:03

XhmikosR force-pushed the stylelint-13 branch from c22219e to d3c286b Compare January 14, 2020 08:09

XhmikosR mentioned this pull request Jan 14, 2020

Stop using private Stylelint code #435

Merged

XhmikosR force-pushed the stylelint-13 branch from 388a983 to 1c13ec5 Compare January 14, 2020 12:50

XhmikosR mentioned this pull request Jan 14, 2020

Switch back to using the lodash package. #437

Merged

bjankord mentioned this pull request Jan 14, 2020

Update dependencies bjankord/stylelint-config-sass-guidelines#61

Merged

Didel mentioned this pull request Jan 17, 2020

npm-warning for "stylelint" peer dependency #441

Closed

XhmikosR force-pushed the stylelint-13 branch 2 times, most recently from fd15d56 to db938d1 Compare January 19, 2020 22:56

XhmikosR mentioned this pull request Jan 19, 2020

Update devDependencies. #444

Merged

XhmikosR force-pushed the stylelint-13 branch from db938d1 to 1ce2ade Compare January 19, 2020 23:09

kristerkari reviewed Jan 22, 2020

View reviewed changes

XhmikosR force-pushed the stylelint-13 branch from 1ce2ade to 4f595de Compare January 23, 2020 06:37

XhmikosR added 2 commits January 23, 2020 08:59

Update stylelint to v13.0.0

aa2e809

Add new upstream files for Stylelint v13.0.0 compatibility.

1445870

XhmikosR force-pushed the stylelint-13 branch from 4f595de to 1445870 Compare January 23, 2020 06:59

kristerkari approved these changes Jan 27, 2020

View reviewed changes

kristerkari merged commit 4ea73e7 into master Jan 27, 2020

XhmikosR deleted the stylelint-13 branch January 27, 2020 13:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update stylelint to v13.0.0 #428

Update stylelint to v13.0.0 #428

XhmikosR commented Jan 12, 2020 •

edited

XhmikosR Jan 12, 2020

XhmikosR commented Jan 12, 2020

hudochenkov commented Jan 12, 2020

XhmikosR commented Jan 12, 2020

hudochenkov commented Jan 12, 2020

XhmikosR commented Jan 12, 2020

hudochenkov commented Jan 12, 2020

XhmikosR commented Jan 12, 2020 •

edited

hudochenkov commented Jan 12, 2020

XhmikosR commented Jan 12, 2020

hudochenkov commented Jan 12, 2020

github-actions bot commented Jan 13, 2020

XhmikosR commented Jan 13, 2020

kristerkari commented Jan 13, 2020

kristerkari commented Jan 13, 2020

XhmikosR commented Jan 14, 2020

XhmikosR commented Jan 14, 2020

XhmikosR commented Jan 19, 2020

kristerkari Jan 22, 2020

XhmikosR Jan 23, 2020 •

edited

XhmikosR Jan 23, 2020

kristerkari Jan 27, 2020

XhmikosR Jan 27, 2020

kristerkari Jan 27, 2020

XhmikosR Jan 27, 2020

pustovalov commented Jan 23, 2020

XhmikosR commented Jan 24, 2020

XhmikosR commented Jan 27, 2020

Update stylelint to v13.0.0 #428

Update stylelint to v13.0.0 #428

Conversation

XhmikosR commented Jan 12, 2020 • edited

XhmikosR Jan 12, 2020

Choose a reason for hiding this comment

XhmikosR commented Jan 12, 2020

hudochenkov commented Jan 12, 2020

XhmikosR commented Jan 12, 2020

hudochenkov commented Jan 12, 2020

XhmikosR commented Jan 12, 2020

hudochenkov commented Jan 12, 2020

XhmikosR commented Jan 12, 2020 • edited

hudochenkov commented Jan 12, 2020

XhmikosR commented Jan 12, 2020

hudochenkov commented Jan 12, 2020

github-actions bot commented Jan 13, 2020

XhmikosR commented Jan 13, 2020

kristerkari commented Jan 13, 2020

kristerkari commented Jan 13, 2020

XhmikosR commented Jan 14, 2020

XhmikosR commented Jan 14, 2020

XhmikosR commented Jan 19, 2020

kristerkari Jan 22, 2020

Choose a reason for hiding this comment

XhmikosR Jan 23, 2020 • edited

Choose a reason for hiding this comment

XhmikosR Jan 23, 2020

Choose a reason for hiding this comment

kristerkari Jan 27, 2020

Choose a reason for hiding this comment

XhmikosR Jan 27, 2020

Choose a reason for hiding this comment

kristerkari Jan 27, 2020

Choose a reason for hiding this comment

XhmikosR Jan 27, 2020

Choose a reason for hiding this comment

pustovalov commented Jan 23, 2020

XhmikosR commented Jan 24, 2020

XhmikosR commented Jan 27, 2020

XhmikosR commented Jan 12, 2020 •

edited

XhmikosR commented Jan 12, 2020 •

edited

XhmikosR Jan 23, 2020 •

edited