slash-no-division

[jrr]

What is the problem you're trying to solve?

My current frontend tooling (w/ Dart Sass) doesn't like division with /:

[CRA] 21 │   padding: 0 ($COL_SPACING / 2);
[CRA]    │               ^^^^^^^^^^^^^^^^
[CRA]    ╵
[CRA]     src/client/styles/modules/Foo.module.scss 21:15  root stylesheet
[CRA]
[CRA] DEPRECATION WARNING: Using / for division is deprecated and will be removed in Dart Sass 2.0.0.
[CRA]
[CRA] Recommendation: math.div($COL-SPACING, 2)
[CRA]
[CRA] More info and automated migrator: https://sass-lang.com/d/slash-div

It currently logs a runtime warning, but presumably in the future it will completely break.

This will probably be caught by our tests, but it would be great if it could also be handled by styelint.

What solution would you like to see?

A new rule that shares the opinion of Dart Sass, squiggling offending uses of the /.

Is this something that stylint-scss would be interested in?

https://sass-lang.com/documentation/breaking-changes/slash-div

(originally from stylelint/stylelint#5368)

[kristerkari]

@jrr Thanks for this! We usually try to avoid adding rules that duplicate the work that the compiler is already doing.

I understand that it would be nice to catch these already before hitting the compiler, but in addition to doing things twice, a big problem is that the rule might get out of sync with the warning that the compiler is giving. That's why I would prefer not to add such rules to the library.

[jrr]

Alright, that makes sense, thanks. I could see the difficulty in maintaining a second implementation that agrees perfectly with the reference implementation.

Maybe one day another extension (perhaps https://github.com/microsoft/vscode-css-languageservice ) could provide squiggles right out of the sass compiler.

[ahukkanen]

I would be in favor of adding this to stylelint-scss.

I wouldn't expect much maintenance against the compiler because the warning will be removed from Dart Sass 2.0.0 as stated in the deprecation warning. So after Dart Sass 2.0.0 the warning is removed, no maintenance is required where as developers could still benefit from the linter warning when migrating their heads to the new style.

Edit: Just for reference, here is the rule in Dart Sass:
https://github.com/sass/dart-sass/blob/a10d7c677dd90f9f5731fa220d1553af620bedea/lib/src/visitor/evaluate.dart#L2100-L2130

[kristerkari]

@ahukkanen I will try to explain my view on this, so maybe that will help you to see why adding such rule does not make sense.

The stylelint-scss is mostly a collection of rules that help to keep your styles consistent by either enforcing some kind of syntax, or disallowing the use of certain features of the language.

For me this kind of rule does not really fall into that normal rule category. The rule would serve as a "migration tool" to the new version of the Sass compiler.

The risk with adding the rule is that there might be false positives or false negatives for what the rule is reporting versus what the compiler warns about, and the rule might get quite complex. Even if it would work perfectly, it seems a bit silly that the rule would just be pointing out the same things that compiler is already telling the user.

In addition to that, the Sass team is already providing all the necessary info and tools to migrate to the new version:

• There is a warning that will point out the exact place that needs to be migrated. Later on that warning will be turned into an error and the code will stop compiling, forcing the migration to be done.
• There is a website that gives all the info and examples about the breaking change: https://sass-lang.com/documentation/breaking-changes/slash-div
• There is an official migration tool that you can use to automatically migrate your styles to the new format to get rid of the warnings: https://sass-lang.com/documentation/breaking-changes/slash-div#automatic-migration

[jrr]

In addition to that, the Sass team is already providing all the necessary info and tools to migrate to the new version:

They're providing everything except for editor squiggles ;)

For better or worse, this project seems to be the default recipient of "there ought to be a squiggle" issues like this one.
Maybe a note on the Readme could map out the ecosystem a little bit, and suggest where to take issues that fall outside this project's scope? (Is there a different project that could/should be responsible for adding VS Code squiggles for that dart-sass warning? I still don't know.)

[kristerkari]

For better or worse, this project seems to be the default recipient of "there ought to be a squiggle" issues like this one.
Maybe a note on the Readme could map out the ecosystem a little bit, and suggest where to take issues that fall outside this project's scope? (Is there a different project that could/should be responsible for adding VS Code squiggles for that dart-sass warning? I still don't know.)

I agree that stylelint provides a much better developer experience than using the compiler for the errors. Even if the rule does not fit stylelint-scss, it would just require a bit of effort to write a custom stylelint plugin that does the same thing.

I have written multiple stylelint plugins, and it would probably take 2-3 hours of work to add the rule, write a test suite, and to add the boilerplate (CI etc.) for the Github repo.

[ahukkanen]

My main concern is the CI pipeline, when we are fixing this dart sass issue for the project. If someone introduces a new slash division after fixing it, it may go unnoticed because there is no way to catch that in the CI.

And there we cannot even grep the webpack output because there are still external dependencies that produce these warnings that are outside of the project's control.

There's still the possibility to write some rather complex webpack output parser that will automatically detect which warnings come from external dependencies and which come from the project itself. Or just hope for the best.

It would be much easier in such cases to rely on stylelint.

[kristerkari]

@ahukkanen what about running the migration tool against your project? https://sass-lang.com/documentation/breaking-changes/slash-div#automatic-migration

[ahukkanen]

@kristerkari Yes, that's what my PR is doing but there are external dependencies that still need to be fixed. And that can take from n days to x years according to my previous experiences.

[kristerkari]

@ahukkanen the problem that you have is a very typical software problem when migrating to a newer version that has a breaking change. If you have external dependencies with breaking changes, then those need to be migrated to the new version regardless of if there is a stylelint rule or not.

So here's my suggestion:

• Run the migration tool against your project.
• Run the migration tool against an external library with the problem and contribute a PR to that library.
• OR build your own stylelint plugin that can be used as tool to find the styles that need migration.

[ahukkanen]

Yep, I understood your point of view @kristerkari.

I was only trying to raise the point that it would be a great help if we had this already incorporated into stylelint.

Based on this discussion it is clear it won't be part of stylelint-scss, I understand that.

[kristerkari]

I also understand how difficult it is to do codebase migrations to newer versions with breaking changes. It takes a lot of time and any kind of tooling will be helpful when doing that.

It's just that stylelint is not a compiler/parser/analyzer, so it should not be used as a tool to validate whether a code compiles or not.

The Sass compiler can have new versions that make some code invalid that was previously working just fine. It's the Sass team that has the compiler/tools that decides which code is valid and which is not, and provide the necessary tools to do the migration to newer versions with breaking changes.