Add no-unknown-custom-properties

[jeddy3]

What is the problem you're trying to solve?

I'd like to disallow unknown custom properties. For example:

:root {
  --foo: 1px;
}

a {
  top: var(--bar);
}

What solution would you like to see?

A new rule to catch this. It's currently available as a plugin but it meets our criteria to be a built-in rule.

Prior art for user-defined unknowns is no-unknown-animations.

Also, see this Twitter thread.

We'd need to address #4088 first, and work out how best to add imports & resolvers to configuration files. Is standardised importForm and resolver secondary options on each of these types of rules the way to go?

• Name: no-unknown-custom-properties
• Primary option: true
• Secondary options: importForm & resolver?
• Autofixable: No
• Message: - Unexpected unknown custom property
• Description: "Disallow unknown custom properties."
• Extended description: "This rule considers custom properties defined within the same source to be known. Use the importForm secondary option to specify more sources."
• Section: "Avoid errors" -> "General"

Like no-unknown-animations, we won't turn the rule on in the recommended config and we'll want to update the recommended config to reflect this.

Any thoughts?

[ybiquitous]

@jeddy3 Thanks for the proposal. I see it as very interesting, and it seems that this feature should be so valuable for many people.

However, as we mentioned many times, how to resolve other files is stuff to be discussed well. It would be good if we could have a PoC that should be the basis of the discussion.

By the way, is importForm a typo of importFrom?

[ybiquitous]

It would be good if we could have a PoC that should be the basis of the discussion.

We may implement such a resolving mechanism as an experimental feature by borrowing code from the existing plugin.

[jeddy3]

By the way, is importForm a typo of importFrom?

Oops, yes it is.

We may implement such a resolving mechanism as an experimental feature by borrowing code from the existing plugin.

That sounds like a sensible approach. And I agree we can flag as experimental for now.

I suggest we start by only supporting .css imports, i.e. importFrom accepts an array of paths to CSS files. Other features like a resolver and supporting other files types, e.g. .html and .scss, can happen later down the line. The latter would likely involve custom syntaxes like we do with overrides. (The existing plugin supports JSON, JS and object imports but let's not go there yet.)

We'd want to author the importFrom option is a way that makes it easier for other rules and plugins to implement.

Do you think there's value in something like a defaultImportFrom configuration property, which would (like defaultSeverity) would sit at the top-level of the configuration object or shall we just go with importFrom as a secondary option on the rule for now?

[ybiquitous]

I suggest we start by only supporting .css imports,

I agree with the idea.

Do you think there's value in something like a defaultImportFrom configuration property

Indeed, there may be a helpful case if defaultImportFrom is supported. But I'm not sure whether it is necessary for the first step. If it is easy to implement defaultImportFrom and importFrom simultaneously, I agree with adding both.

[jeddy3]

Indeed, there may be a helpful case if defaultImportFrom is supported. But I'm not sure whether it is necessary for the first step. If it is easy to implement defaultImportFrom and importFrom simultaneously, I agree with adding both.

I agree.

I'll label this as ready to implement.

• Name: no-unknown-custom-properties
• Primary option: true
• Secondary options: importFrom: [] - array of glob paths to .css files
• Autofixable: No
• Message: - Unexpected unknown custom property
• Description: "Disallow unknown custom properties."
• Extended description: "This rule considers custom properties defined within the same source to be known. You can use the importFrom secondary option to specify more sources."
• Section: "Avoid errors" -> "Unknown"

If anyone fancies contributing this rule, there are steps on how to add a new rule in the Developer guide.

[silverwind]

Regarding importFrom: Yes, sounds sensible to start off with only supporting manually declared CSS sources.

If a flat config is implemented, variable declarations could be automatically extracted from all files in the same "job", which then should be able to rely on user-provided customSyntax parsers for HTML/SCSS etc.

[jameschensmith]

Thanks @silverwind for connecting the tickets together, which helped get my attention. I'm totally on board with this, and will be happy to implement this here (this will be my first PR in this repository, so I'll be following the developer guide @jeddy3 mentioned). I have one question, though. Why not just implement it without secondary options for now, and POC the importFrom option separately? I'm fine implementing it either with/without the second option. I just think if we iterate on this rule, we can deliver value much more quickly.

Once I hear back on what y'all think, I can start iterating on it.

[ybiquitous]

Why not just implement it without secondary options for now, and POC the importFrom option separately?

Sounds good. I welcome a pull request to implement the new rule without the secondary option.

[jameschensmith]

PR for initial implementation can be found here.

[silverwind]

Implementation was merged, so I assume we can close this one.

Should we make a separate issue for the feature request for a importFrom option?

[ybiquitous]

Should we make a separate issue for the feature request for a importFrom option?

Surely, leaving this issue title as-is may confuse people. Let's close this after we open a new follow-up issue.

[ybiquitous]

I just opened #6746. This can be closed.