Add a check for interpolated variables to node-sass workflow and fix wrong variables

[julien-deramond]

Description

This PR closes #38278:

• Add a step to check if there are no Sass variables in the CSS files built via node-sass in our GitHub workflow
  • Without any fixes, it fails: https://github.com/twbs/bootstrap/actions/runs/4466765702/jobs/7845359888?pr=38283
  • With the fix, it passes: https://github.com/twbs/bootstrap/actions/runs/4466887769/jobs/7845646876?pr=38283
• Use interpolation for each use case
• Check that there's no regression
• Check if Stylelint can help (Add a check for interpolated variables to node-sass workflow and fix wrong variables #38283 (comment))

Type of changes

• Bug fix (non-breaking change which fixes an issue)

Checklist

• I have read the contributing guidelines
• My code follows the code style of the project (using npm run lint)
• (N/A) My change introduces changes to the documentation
• (N/A) I have updated the documentation accordingly
• (N/A) I have added tests to cover my changes
• All new and existing tests passed

Related issues

Closes #38278

[danny007in]

better to implement stylelint

[julien-deramond]

better to implement stylelint

Do you mean to check our Sass files to avoid to forget any interpolations, or in the GitHub workflow that tries the compilation with node-sass (which isn't the default one)? Or both?

[danny007in]

better to implement stylelint

Do you mean to check our Sass files to avoid to forget any interpolations, or in the GitHub workflow that tries the compilation with node-sass (which isn't the default one)? Or both?

Yes, Check https://github.com/stylelint-scss/stylelint-scss/tree/master/src/rules/dollar-variable-no-missing-interpolation

[julien-deramond]

Yes, Check https://github.com/stylelint-scss/stylelint-scss/tree/master/src/rules/dollar-variable-no-missing-interpolation

Just tried it.

The following use case generates an error like explained in the example:

.class {
  $var: "my-anim";
  animation-name: $var;
//                ↑
// This variable needs to be interpolated
// because its value is a string
}

But margin-left: calc($btn-border-width * -1); isn't. Unless I'm missing something which is possible, this solution doesn't seem to help in this use case.

[XhmikosR]

@julien-deramond maybe we should split the fixes to a new PR so that we merge it and then merge the CI change?

[julien-deramond]

Having everything in the same place, for now, is easier for me right to have a global view.
Once everything is checked on my side (check that there is no regression with these interpolation fixes), I can of course split the PR, no problem.

[kristerkari]

But margin-left: calc($btn-border-width * -1); isn't. Unless I'm missing something which is possible, this solution doesn't seem to help in this use case.

@julien-deramond @danny007in dollar-variable-no-missing-interpolation does not to handle calc, and the rule could be fixed to do that, but it probably is not able to report the need for interpolation unless the variable is defined in the same file as the calc.

[danny007in]

Check stylelint-scss/stylelint-scss#786

[mdo]

This looks great to me, @julien-deramond! Feel free to merge whenever.

[julien-deramond]

I've double-checked a part of the use cases. Interpolation is working well. Since @mdo has already reviewed it, let's

[XhmikosR]

It should have been split into separate PRs... Anyway, keep it on mind for the future.

Because the Sass changes were real bugs and unrelated to the CI addition.