Bring back warning option

[danny-andrews-snap]

I'm gonna go out on a limb here and say that the warning option should be brought back. I understand that you think it is the job of a linter, but I kindly disagree. :) Follow with me, and please correct me if I'm wrong on any of these points:

1. Anyone using this plugin is developing for at least one browser which does not support custom properties.
2. If any undefined properties are encountered, that bit of CSS won't work on said browser(s).
3. This is a bug which the user should be informed about as early as possible.

Why leave this type of thing up to a linter? Just this week I ran into an issue where custom properties were not being replaced, and didn't realize it until trying my code in IE. The warning option already defaulted to false, so it didn't bother anyone who didn't specifically opt-in to it, so why remove it for those who want to use it?

From the release notes:

Changed: warnings option defaults to false to reflect the browser climate

I appreciate the forward-thinkingness, but if you think the browser climate is so good, shouldn't you just deprecate this plugin as it's not needed anymore? 🤷‍♂️

The majority of developers have to write code for browsers without custom properties support. Why not give them hints about conditions which could break their code?

Thanks!

Related #153, #117.

[jonathantneal]

Hey there, I understand your concerns. I do recommend that you use a linter. It sounds like it would have resolved your issues. I’m not sure why you would want these things linted while you would not want a linter to tell you.

You’ve made some assumptions about how folks use Custom Properties, which I think are fair assumptions if you were accustomed to Sass variables, but do not necessarily apply to the dynamic abilities of Custom Properties.

Anyone using this plugin is developing for at least one browser which does not support custom properties.

It is recommended that this plugin be used with a browserslist, and to be forward-compliant, it gracefully allows spec-compliant variables even if they cannot be detected. However, a fair number of PostCSS Preset Env users are not providing Custom Property fallbacks for IE, because their browserslist does not include IE.

If any undefined properties are encountered, that bit of CSS won't work on said browser(s).

An unmatched Custom Property will not break any rule in CSS. Custom Properties can and do sit around waiting for something to activate them, usually via JavaScript.

This is a bug which the user should be informed about as early as possible.

This isn’t necessarily so, if the Custom Properties are coming from non-PostCSS styles or third-party styles or if they are activated via JavaScript.

[danny-andrews-snap]

However, a fair number of PostCSS Preset Env users are not providing Custom Property fallbacks for IE, because their browserslist does not include IE.

In that case, wouldn't PostCSS Preset Env just exclude this plugin altogether? Why perform redundant work, and duplicate custom property values if you are developing for a browser which supports them without transformation?

[jonathantneal]

I’m always willing to listen to new ideas and arguments, as well as rehash old ones, but I consider it bad form to open new issues and also critique what I’m saying on dead issues.

I do see that you commented there and then here. Thank you for correcting me. I need to figure out how to lock dead issues, as I errantly became very worried you would create multiple threads for me to respond to.

[jonathantneal]

I’m re-opening the issue so others can contribute. I still feel the same way that your suggestion is best covered by a linter, but I don’t want my misunderstanding to get in the way of proper discussions and development.

[danny-andrews-snap]

I understand the concern. :) OSS development is a thankless job, and developers can be entitled brats.

[privatenumber]

I think the warnings should be brought back as well.

Since this plugin statically compiles the variables, if the plugin can't find it, it should emit a warning.

I think the linting rule is appropriate for catching missing variables when using the native custom-properties feature (dynamic CSS variables), but isn't appropriate to catch compilation errors.

[jonathantneal]

I need to remind you all that it’s not a compilation error. You don’t have to have Custom Properties defined to use them.

[danny-andrews-snap]

But again, why would you be using this plugin if you are developing for a browser which supports custom properties natively?

I think in cases like this, it helps to think of the average use-case. I can't say for sure, but I would reckon that the majority of users of this plugin are developing for at least one browser which doesn't support native custom properties. As such, if they have a custom property lacking a definition, they will get unexpected behavior when they deploy their code (e.g. why isn't this font --my-custom-color?). Giving them the ability to be warned for properties with no matching definition helps them to catch this error case early.

[stefanmaric]

@jonathantneal I was looking around for info on how to report (warn or bail) the use of undefined properties (with or without fallback) and also the declaration of unused ones.

Do you know the existence of such rules/plugins for stylelint?

If not, is it even possible? — How would a stylelint plugin match property declarations to var() invocations across @imports (assuming you're using the postcss-import plugin for your css compilation) and the files imported with importFrom? I would guess the stylint plugin would have to replicate all the logic of postcss-custom-properties and the user would have to take great care to keep the configs in sync.

I feel these features are worth to be included here because they are very implementation-specific.

[jonathantneal]

All of this is possible with:

https://www.npmjs.com/package/stylelint-value-no-unknown-custom-properties

And it uses the same style of options as this plugin.

[stefanmaric]

https://www.npmjs.com/package/stylelint-value-no-unknown-custom-properties

Alright, it is implemented as I expected. Not ideal, but works nonetheless. :smile:

I will try to create one for the unused ones. Or does it exist already?