Fix value-no-vendor-prefix false negatives

[Mouvedia]

While working on a fix I realised how the rule was intended to work. We can't change that behaviour after the fact.
It reports if the value can be unprefixed safely, else it bails-out.

In practice this is what you want and actually that's better for us: we don't have to update stylelint-config-standard with an -apple-system exception anymore because it is not an unprefixable value.

---

What minimal example or steps are needed to reproduce the bug?

body {
  font: 16px/1.2 BlinkMacSystemFont, -apple-system, "Segoe UI", Roboto, Helvetica, Arial, sans-serif;
}

What minimal configuration is needed to reproduce the bug?

{
  "rules": {
    "value-no-vendor-prefix": true
  }
}

How did you run Stylelint?

https://stylelint.io/demo

Do you have a proposal to fix the bug?

replace

stylelint/lib/reference/prefixes.mjs

Line 1 in 9959110

export const prefixes = new Set(['-webkit-', '-moz-', '-ms-', '-o-']);

by

export const prefixes = new Set(['-webkit-', '-moz-', '-ms-', '-o-', '-konq-', '-apple-', '-khtml-']);

Resources

• Fixed selector|value-no-vendor-prefix performance #7016 (review)
• https://booking.design/implementing-system-fonts-on-booking-com-a-lesson-learned-bdc984df627f
• https://webkit.org/blog/3709/using-the-system-font-in-web-content/

Caveat

system will have to be added to

https://github.com/stylelint/stylelint-config-standard/blob/3c610c147cc6c20b9e24292ff002b4637d55d3bc/index.js#L117

because it's used in many code bases as part of the default system font stack values.
Other values, like -apple-system-body, are rarely used, so it doesn't matter as much.

---

There are 64 values that start with "-apple-system" supported by Safari.
see https://github.com/WebKit/WebKit/blob/main/Source/WebCore/css/CSSValueKeywords.in
Hence being able to ignore them using a regex is essential.
e.g. ["/^-apple-system/"]
So this fix will have to be introduced alongside an update of ignoreValues.
e.g.

from

ignoreValues: ["string"]

to

ignoreValues: ["/regex/", /regex/, "string"]

[ybiquitous]

@Mouvedia Thanks for the report. Supporting the -apple- prefix sounds good to me. 👍🏼

And, adding regex support to the ignoreValues secondary option of value-no-vendor-prefix seems no problem. 👍🏼

Just to be sure, I have a few questions:

• Adding -apple- to reference/prefixes.mjs will affect selector-no-vendor-prefix, but are there any -apple- selectors? And, if there are (e.g. :-apple-foo-bar), will this addition fix also selector-no-vendor-prefix false negatives?

  • stylelint/lib/rules/selector-no-vendor-prefix/index.mjs

    Line 2 in 9959110

    import hasPrefix from '../../utils/hasPrefix.mjs';

• You suggest adding also -konq- and -khtml-, but what is rationale to add them, too?

[Mouvedia]

…but are there any -apple- selectors?

If you are talking about properties there are 3 AFAIK:

• -apple-dashboard-region
• -apple-line-clamp
• -apple-text-size-adjust

…will this addition fix also selector-no-vendor-prefix false negatives?

It will but it's not problematic. The problem is that this fix requires to update the ignoreValues option of value-no-vendor-prefix. It's similar to #7232 in the sense that a fix must be accompanied by its cure or it would be a downgrade.
And depending on our policies, updating ['string'] might require to wait for the next major.

There are 2 alternatives:

1. technically we could have 2 export for each rule (only change selector-no-vendor-prefix before v17)
2. add ["/regex/", /regex/] and wait for v17 to update ['string'] (make it exact match)

I vote for 2 because you can do exact match using regexes too. If so a new issue will have to be created akin to #7542.

tl;dr: the fix for selector-no-vendor-prefix is uncomplicated, for value-no-vendor-prefix it requires to update stylelint-config-standard and its secondary option.

…but what is rationale to add them, too?

Adding all the prefixes that ever existed makes sense.
At least that's what I would expect from *-no-vendor-prefix rules.

[ybiquitous]

Adding all the prefixes that ever existed makes sense.
At least that's what I would expect from *-no-vendor-prefix rules.

Okay. For -konq- and -khtml-, are there any resources? If yes, it seems good to leave a comment with links to such resources in reference/prefixes.mjs.

EDIT: Because many people may not know the -apple-, -konq-, and -khtml- prefixes.

[ybiquitous]

And depending on our policies, updating ['string'] might require to wait for the next major.

Just to confirm, does this update mean the following in my previous comment (#7624 (comment))? 👇🏼

adding regex support to the ignoreValues secondary option of value-no-vendor-prefix

[Mouvedia]

…are there any resources?

Yes, Iv linked to it in #7016 (review)

…it seems good to leave a comment with links to such resources in reference/prefixes.mjs.

👍

And depending on our policies, updating ['string'] might require to wait for the next major.

Just to confirm, does this update mean the following in my previous comment (#7624 (comment))? 👇🏼

adding regex support to the ignoreValues secondary option of value-no-vendor-prefix

If I did the PR, I would add support for ["/regex/", /regex/] without updating ["string"].
i.e. "a new issue will have to be created akin to #7542" to cover the breaking change

[ybiquitous]

Sorry, I can't still understand what you mean... It seems that it's not breaking to change ignoreValues: ["string"] to ignoreValues: ["/regex/", /regex/, "string"] 🤔

[Mouvedia]

It depends on how you proceed :)

Currently ["string"] picks up prefixes by default.
e.g. ['foo'] will match -webkit-foo
That's undesirable and unexpected and ["/regex/", /regex/] won't work that way.
e.g. stylelint-config-standard currently is ignoring more than it should
see stylelint/stylelint-config-standard#261 (comment)

So until the next major there will be inconsistencies between ["/regex/", /regex/] and ["string"].
Is that clearer?

[ybiquitous]

Ah, I now understand the problem here. 😅

The current ignoreValues option of value-no-vendor-prefix works on unprefixed values, but we'd like to support regex for a whole value (e.g. /^-apple-system/).

stylelint/lib/rules/value-no-vendor-prefix/index.mjs

Lines 55 to 57 in 9959110

	if (optionsMatches(secondaryOptions, 'ignoreValues', vendor.unprefixed(value))) {
	return;
	}

Indeed, this may a breaking change. So, cannot we add the regex support, keeping compatibility? 🤔

E.g.

 if (optionsMatches(secondaryOptions, 'ignoreValues', vendor.unprefixed(value))) { 
 	return; 
 }

+if (optionsMatches(secondaryOptions, 'ignoreValues', value)) { 
+	return; 
+}

[Mouvedia]

You can probably parse ignoreValues values.
Anyhow that's up to the implementor to decide.

[ybiquitous]

You can probably parse ignoreValues values.

Hum, I'd like to avoid parsing the option values since it seems too complicated... 🤔

[Mouvedia]

@ybiquitous Should I create another issue for ["/regex/", /regex/]?
If not is the secondary option update ready to be implemented?

[ybiquitous]

@Mouvedia Yes, please create another issue. For the secondary option, I guess:

• Add regex support. Specified regex should match a whole value rather than an unprefixed value. E.g. ignoreValues: [/^-apple-/] should match -apple-system or -apple-foo etc.
• Show a warning if a string is included in ignoreValues. E.g. String values in "ignoredValues" for "value-no-vendor-prefix" will match a whole declaration value rather than a unprefixed value in the next major version. If you want to keep the current behavior, please consider using a regex
• Update stylelint-config-standard to rewrite the string values to regex ones for value-no-vendor-prefix

After the processes completes, we can fix false negatives reported in this issue. Any thoughts?

[Mouvedia]

please create another issue

Done.

Show a warning if…

I will only do it if the value starts with a known prefix.

Update stylelint-config-standard to rewrite the string values to regex ones for value-no-vendor-prefix
After the processes completes, we can fix false negatives reported in this issue. Any thoughts?

stylelint-config-standard will have to be updated twice: once for string to regex and once for the "/^-apple-system$/" addition. I would prefer to wait for both issues to be fixed on stylelint/stylelint to move on with the PR.

[ybiquitous]

Agree. We'd like to release stylelint and stylelint-config-standard only once.