Skip to content

fix: var() with fallback value in no-invalid-properties #184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Tanujkanti4441
Copy link
Contributor

Prerequisites checklist

What is the purpose of this pull request?

To fix the lining errors while using var() with fallback values.

What changes did you make? (Give an overview)

Added a function to remove fallbacks when the variable value is present and a function to get a list of fallback values to use it, when there is no reference present of the variable.

So these cases are the valid:

:root { --my-fallback: red; }

a { color: var(--my-color, red) }

a { color: var(--my-color, var(--my-fallback)) }

:root { --my-color: red; }

a { color: var(--my-color, var(--fallback-color, blue)) }

Related Issues

fixes #180

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

@eslint-github-bot eslint-github-bot bot added the bug Something isn't working label Jun 30, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Jun 30, 2025
@nzakas nzakas moved this from Needs Triage to Implementing in Triage Jun 30, 2025
@lumirlumir lumirlumir added the accepted There is consensus among the team that this change meets the criteria for inclusion label Jul 1, 2025
@@ -37,6 +37,23 @@ ruleTester.run("no-invalid-properties", rule, {
":root { --my-color: red; }\na { color: var( --my-color ) }",
":root { --my-color: red;\n.foo { color: var(--my-color) }\n}",
".fluidHeading {font-size: clamp(2.1rem, calc(7.2vw - 0.2rem), 2.5rem);}",
"a { color: var(--my-color, red) }",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some invalid tests that use fallbacks with incorrect or missing values? Based on the code changes in this PR, I think the error locations will be off.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems you are right, I think it still needs some work, like multiple var in the property and this, so marking it as draft for now. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have updated the code to fix this and added some tests as well?

@Tanujkanti4441 Tanujkanti4441 marked this pull request as draft July 4, 2025 15:57
@Tanujkanti4441 Tanujkanti4441 marked this pull request as ready for review July 6, 2025 18:05
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice job. Would like another review before merging.

@nzakas nzakas moved this from Implementing to Second Review Needed in Triage Jul 7, 2025
@snitin315
Copy link
Contributor

@Tanujkanti4441 There are few ci failures, can you please check once?

@nzakas
Copy link
Member

nzakas commented Jul 7, 2025

The failures have already been fixed on main: #191

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion bug Something isn't working
Projects
Status: Second Review Needed
Development

Successfully merging this pull request may close these issues.

Bug: error when defining fallbacks in var in no-invalid-properties
4 participants