feat: baseline rule #33

nzakas · 2024-12-12T17:42:45Z

Prerequisites checklist

I have read the contributing guidelines.

What is the purpose of this pull request?

Create a rule that warns when non-baseline features are used.

What changes did you make? (Give an overview)

Added baseline rule
Added tests and docs
Created a tool to generate baseline data

Related Issues

fixes #26

Is there anything you'd like reviewers to focus on?

I'm not 100% sure that the baseline data matches what's on the website. The web-features package doesn't tag things as "widely" or "newly", just "high", "low", or false. I started with the assumption that "high" means "widely" and "low" means "newly", but I'm not sure that's correct. high = widely, low = newly
I set this as recommended, as it seems like it would generally be useful, but definitely open to feedback on that.

jamesnw · 2024-12-12T18:15:36Z

I'm not 100% sure that the baseline data matches what's on the website. The web-features package doesn't tag things as "widely" or "newly", just "high", "low", or false. I started with the assumption that "high" means "widely" and "low" means "newly", but I'm not sure that's correct.

Web Features contributor here- your assumption is correct.

low = Newly available: The feature works across the latest devices and browser versions. The feature might not work in older devices or browsers.

high = Widely available: The feature is well established and works across many devices and browser versions. It’s been available across browsers for at least 2½ years (30 months).

false = Limited availability: Some but not all of the latest devices and browser versions support it, and it might not work, even in up to date devices and browsers.

Thanks for doing this!

jamesnw

This is really exciting to see!

I added a few comments that may help with feature detection. The keys that are used in the web-features package are defined by BCD. While I think they can get you pretty close on feature detection, that isn't the intended use necessarily, and I suspect there will be some edge cases.

jamesnw · 2024-12-12T18:54:28Z

src/data/baseline-data.js

+	["color", 10],
+	["color-scheme", 10],
+	["break-after", 0],
+	["break-after.multicol_context", 0],


This is an example of a contextual modifier on the BCD key- the string "multicol_context" won't show up in CSS, but this modifier denotes that the break-after would work inside a layout that using column.

The convention is that these keys will have underscores, and this potentially could be useful down the road to further target linting. However, CSS properties are going to never have a period, so I think the regex could be adjusted to drop the .multicol_context, or anything after a period. Essentially, anything beginning with css.properties, you could do something like key.split('.')[2] to get the property name.

Ah, that's very helpful, thanks. Yeah, I left these because I was trying to figure out if I could do something with the values after the period.

jamesnw · 2024-12-12T18:56:19Z

src/data/baseline-data.js

+	["custom-property", 10],
+	["display", 10],
+	["display.none", 10],
+	["display.contents", 0],


display.contents signifies a rule like this-

a {display: contents}. If there is not an underscore in the second portion, it is the value. It would be nice to be able to check for this as well.

Thanks for explaining this. I looked into it a bit and this is more complicated than it seems. Sometimes the identifier after the property name is a literal (as in display.content means display: contents), but sometimes it's a type (content.url means content: url(foo)). So it's not enough to just pull that value and check for it, we'd also need to know what type of node to look for in CSSTree.

And it gets even more complicated with types that actually represent multiple values, like border-radius.percentages, which may be represented by 1-4 nodes. I suppose we could filter and just check on the properties that are known to only have literal values, but I don't know how to automate that.

Ultimately, it looks like sussing out all of these details would be a labor-intensive process, so it's unlikely to happen without outside contributions.

jamesnw · 2024-12-12T19:01:54Z

src/data/baseline-data.js

+	["namespace", 10],
+	["page", 0],
+	["page.size", 0],
+	["media.prefers-color-scheme", 10],


This would indicate a rule like

@media (prefers-color-scheme: dark){}.

jamesnw · 2024-12-12T19:13:20Z

src/data/baseline-data.js

+	["starting-style", 0],
+	["supports", 10],
+]);
+export const types = new Map([


I noticed you're not testing types yet, so this is more of a note for the future.

Types are going to be tricky to detect from the key. Some will be fairly straightforward- abs, sign, anchor, for instance, will all show up as functions with that name, in a value in an AST.

A few items like gradient.* and easing-function.* would show similarly show up as functions if you took the substring after the ..

Others like time or string would not show up literally, but would need to be detected in a different way.

Yeah, I figured for the first version of the rule I'd punt on this, but I wanted the data here for easy reference later.

The only way I can think to make this work is to maintain a separate dictionary that maps types to their expected AST locations.

jamesnw · 2024-12-12T19:17:48Z

src/data/baseline-data.js

+	["percentage", 10],
+	["integer", 10],
+]);
+export const selectors = new Map([


For selectors, feature detection could be fairly straightforward. Some like :has would have a single : and show up in the AST as PseudoClassSelector, others like ::before would have 2 :: and show up in the AST as a PseudoElementSelector.

I don't think there's a way to differentiate between the two classes based on this data.

There are also some items here that are not actually verbatim selectors- for instance, "class" refers to the . notation for selecting classes.

Yeah, that's the challenge I ran into, so similar to types, I figured I'd just make the data was there and I'd come back to it later. Probably will need to be addressed similar to types where there needs to be a separate dictionary indicating which are pseudoclasses vs. pseudoelements vs. other.

nzakas · 2024-12-16T19:50:17Z

Okay, I think I've got this in pretty good shape now.

Properties and identifier values are now validated against baseline.
You can use @supports to indicate you know what you're doing so the rule doesn't flag its contents. (Also works with nested @supports).
There is some rudimentary checking of function values against baseline.

I'd love for folks to try this out and give me some feedback on if it's helpful.

FYI: I'm now offline until December 30, but please feel free to leave your feedback and I'll review when I return. Happy holidays!

nzakas · 2025-01-02T19:19:00Z

Okay, I think this rule is ready to go! Some updates:

The rule now checks for identifier values, color names, and function names. It's not all possible values but it's a start.
@supports now works for properties and functions. Inside of an @supports block, you can use whatever is specified as the condition regardless of its baseline status. The rule omits checking anything preceded by not for best compat.

Open question for me: Is baseline the right name? Should it maybe be use-baseline or require-baseline instead?

fixes #26

JoshuaKGoldberg

🔥 Core functionality all looks great! The logic encapsulation in the classes made it pretty straightforward to review functionally.

Only one blocking comment: the [Bug] around _ in names. If that is fixed then ✅ from me.

Left some other suggestions too, but nothing I think is critical.

JoshuaKGoldberg · 2025-01-22T12:33:59Z

docs/rules/baseline.md

@@ -0,0 +1,71 @@
+# baseline
+
+Enforce the use of baseline features


[Docs] This reads to me as the opposite of what the rule does. I see the rule as being in the family of "Disallow ..." because it reports to stop devs from using certain syntax.

Suggesting making the description more in line with that:

Suggested change

Enforce the use of baseline features

Disallow use of CSS not in the Baseline available feature set

We only use "disallow" when the rule begins with "no". For others, we use "Enforce" (or "Suggest").

docs/rules/baseline.md

package.json

JoshuaKGoldberg · 2025-01-22T12:40:17Z

src/data/colors.js

[Refactor] Any reason not to use a package such as css-color-names? This feels like something that should come from a more centralized source.

If a good one doesn't exist -I couldn't find a package that exports a Set- it feels like a good opportunity to file a followup for publishing a new package.

I don't like adding new dependencies for a dataset this small and easy to manage. Seems like unnecessary overhead and adding a blocking dependency where it's not absolutely necessary.

On creating a new package: maybe. I think it's too early to be thinking about extracting parts of this plugin into separate packages. Not saying it won't happen, I just don't think we have a good idea of what we'll be managing yet.

JoshuaKGoldberg · 2025-01-22T12:42:12Z

src/rules/baseline.js

+		const supportedProperty = this.#properties.get(property);
+
+		if (!supportedProperty) {
+			return false;
+		}
+
+		return supportedProperty.hasIdentifier(identifier);


[Style] Nit, just a bit of code golf:

Suggested change

const supportedProperty = this.#properties.get(property);

if (!supportedProperty) {

return false;

}

return supportedProperty.hasIdentifier(identifier);

return !!this.#properties.get(property)?.hasIdentifier(identifier);

The same refactor could be done in the other methods that check if (!supportedProperty) {.

We don't do nits on our PR code reviews. 😄

And I prefer my version as I think it's a lot easier to read and understand.

JoshuaKGoldberg · 2025-01-22T12:46:58Z

tools/generate-baseline.js

+// export each group separately as a Set, such as highProperties, lowProperties, etc.
+const code = `/**
+ * @fileoverview CSS features extracted from the web-features package.
+ * @author tools/generate-baseline.js


[Non-Actionable] TIL the syntax highlighting for author is sensitive to a slash, interesting:

Additional TIL that there's no TypeScript->editor-native way to link to another file. microsoft/TypeScript#47718

🤷 I don't know of a better way to say this is authored by another file. So, not requesting changes, just ... vague interest.

JoshuaKGoldberg · 2025-01-22T13:09:59Z

docs/rules/baseline.md

+
+This rule accepts an option object with the following properties:
+
+- `available` (default: `"widely"`) - change to `"newly"` available to allow a larger number of properties and at-rules.


[Docs] I think it'd be good to be a bit more precise. Many folks reading this file probably won't be confident enough to piece it together. Maybe...

Suggested change

- `available` (default: `"widely"`) - change to `"newly"` available to allow a larger number of properties and at-rules.

- `available` (default: `"widely"`) - change to `"newly"` available to allow a larger number of properties and at-rules per [How do things become Baseline?](https://web.dev/baseline#how-do-things-become-baseline).

I couldn't find a better single linkable resource on the web.dev site...

I don't mind adding a reference, but it should be a "Further Reading" section, not inline with the option.

JoshuaKGoldberg · 2025-01-22T13:41:08Z

tools/generate-baseline.js

+		/^css\.properties\.(?<property>[a-zA-Z$\d-]+)\.(?<value>[a-zA-Z$\d-]+)$/u;
+	const cssAtRulePattern = /^css\.at-rules\.(?<atRule>[a-zA-Z$\d-]+)$/u;
+	const cssTypePattern = /^css\.types\.(?<type>[a-zA-Z$\d-]+)$/u;
+	const cssSelectorPattern = /^css\.selectors\.(?<selector>[a-zA-Z$\d-]+)$/u;


[Bug] These don't allow _ in the matched named groups. Seems like an accidental omission in the data?

> /^css\.properties\.(?<property>[a-zA-Z$\d-]+)$/u.test("css.properties.language_welsh") false > /^css\.properties\.(?<property>[_a-zA-Z$\d-]+)$/u.test("css.properties.language_welsh") true

I also found these regular expressions to be a bit hard to read through at first. Really all they're doing is splitting on . and checking the next 2-3 elements after the css., right?

Suggestion 1 (testing): add an invalid and valid test for one of the names with a _, to make sure they get captured.

Suggestion 2 (refactor): I tried out switching them to a const [, group, area, ...rest] = key.split("."); and that got all the _ keys into the data without failing any tests. I think this'd be a bit easier to work with.

for (const [key, baseline] of Object.entries(features)) { const [, group, area, ...rest] = key.split("."); switch (group) { case "properties": switch (rest.length) { case 0: properties[area] = baselineIds.get(baseline); break; case 1: propertyValues[area] ||= {}; propertyValues[area][rest[0]] = baselineIds.get(baseline); break; } break; case "at-rules": if (!rest.length) { atRules[area] = baselineIds.get(baseline); } break; case "types": if (!rest.length) { types[area] = baselineIds.get(baseline); } break; case "selectors": if (!rest.length) { selectors[area] = baselineIds.get(baseline); } break; } }

Good catch! This is actually intentional and not a bug.

The way that the syntax is described, only descriptions have underscores in them, such as css.properties.align-self.position_absolute_context. There's no valid CSS syntax (in the spec) that contains underscores.

I'll add a comment to this effect.

tools/generate-baseline.js

JoshuaKGoldberg · 2025-01-22T13:56:04Z

docs/rules/baseline.md

@@ -0,0 +1,71 @@
+# baseline


Open question for me: Is baseline the right name? Should it maybe be use-baseline or require-baseline instead?

[Naming] Maybe, baseline-support or baseline-supported, to indicate it's about specifically the support aspect of baseline? But that disambiguation is probably only useful if there are other baseline-related rules to disambiguate from. And there are none. So no strong preference here.

JoshuaKGoldberg

(wrong button)

Co-authored-by: Josh Goldberg ✨ <[email protected]>

eslint-github-bot bot added the feature label Dec 12, 2024

nzakas mentioned this pull request Dec 12, 2024

New Rule: baselineWidelyAvailable #26

Open

jamesnw reviewed Dec 12, 2024

View reviewed changes

nzakas added 10 commits January 21, 2025 13:34

feat: baseline rule

f9935b4

fixes #26

Add rule options to docs

7521367

Omit data file from prettier check

37d167f

Don't rebuild baseline on every build

dae9f8e

Update to check some property values

0eed212

Nested @supports rules support

a3f90b3

Add basic checking for functions

a2626a9

Fix style issues

197260b

Fix baseline edge cases

a4a6194

Honor not in @supports

5f1f770

nzakas force-pushed the issue26 branch from 0b14dd4 to 5f1f770 Compare January 21, 2025 18:36

Fix formatting errors

3bbc969

JoshuaKGoldberg approved these changes Jan 22, 2025

View reviewed changes

JoshuaKGoldberg requested changes Jan 22, 2025

View reviewed changes

nzakas and others added 8 commits January 22, 2025 13:07

Update docs/rules/baseline.md

4d936e7

Co-authored-by: Josh Goldberg ✨ <[email protected]>

Update package.json

c1ce678

Co-authored-by: Josh Goldberg ✨ <[email protected]>

Update tools/generate-baseline.js

7edcd62

Co-authored-by: Josh Goldberg ✨ <[email protected]>

Add link to baseline docs

cdf3861

Add comment to generate-baseline script

7ccc9cf

Fix missing ref

cef99e5

Update prettierignore

c3c410c

Fix colors formatting

ab1fd70

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: baseline rule #33

feat: baseline rule #33

nzakas commented Dec 12, 2024 •

edited

Loading

jamesnw commented Dec 12, 2024 •

edited

Loading

jamesnw left a comment

jamesnw Dec 12, 2024

nzakas Dec 12, 2024

jamesnw Dec 12, 2024

nzakas Dec 13, 2024

jamesnw Dec 12, 2024

jamesnw Dec 12, 2024

nzakas Dec 12, 2024

jamesnw Dec 12, 2024

nzakas Dec 12, 2024 •

edited

Loading

nzakas commented Dec 16, 2024

nzakas commented Jan 2, 2025

JoshuaKGoldberg left a comment

JoshuaKGoldberg Jan 22, 2025

nzakas Jan 22, 2025

JoshuaKGoldberg Jan 22, 2025

nzakas Jan 22, 2025

JoshuaKGoldberg Jan 22, 2025

nzakas Jan 22, 2025

JoshuaKGoldberg Jan 22, 2025

JoshuaKGoldberg Jan 22, 2025

nzakas Jan 22, 2025

JoshuaKGoldberg Jan 22, 2025

nzakas Jan 22, 2025

JoshuaKGoldberg Jan 22, 2025

JoshuaKGoldberg left a comment

		@@ -0,0 +1,71 @@
		# baseline

		Enforce the use of baseline features

	Enforce the use of baseline features
	Disallow use of CSS not in the Baseline available feature set


		This rule accepts an option object with the following properties:

		- `available` (default: `"widely"`) - change to `"newly"` available to allow a larger number of properties and at-rules.

feat: baseline rule #33

Are you sure you want to change the base?

feat: baseline rule #33

Conversation

nzakas commented Dec 12, 2024 • edited Loading

Prerequisites checklist

What is the purpose of this pull request?

What changes did you make? (Give an overview)

Related Issues

Is there anything you'd like reviewers to focus on?

jamesnw commented Dec 12, 2024 • edited Loading

jamesnw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nzakas Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

nzakas commented Dec 16, 2024

nzakas commented Jan 2, 2025

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

nzakas commented Dec 12, 2024 •

edited

Loading

jamesnw commented Dec 12, 2024 •

edited

Loading

nzakas Dec 12, 2024 •

edited

Loading