Skip to content
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

**:not()** with a selector list doesn't expand & symbol #4313

Open
WORMSS opened this issue Jan 30, 2025 · 6 comments
Open

**:not()** with a selector list doesn't expand & symbol #4313

WORMSS opened this issue Jan 30, 2025 · 6 comments
Labels

Comments

@WORMSS
Copy link

WORMSS commented Jan 30, 2025

:not(&--a):not(&--b) does expand the & symbol into the parent's selector as expected.
:not(&--a, &--b) doesn't expand the & symbol into the parent's selector.

To reproduce:

.works {
  &:not(&--a):not(&--b):hover {
    background-color: green;
  }
}

.broken {
  &:not(&--a, &--b):hover {
    background-color: green;
  }
}

Current behavior:

.works:not(.works--a):not(.works--b):hover {
  background-color: green;
}

.broken:not(&--a, &--b):hover {
  background-color: green;
}

Expected behavior:

.works:not(.works--a):not(.works--b):hover {
  background-color: green;
}

.broken:not(.broken--a, .broken--b):hover {
  background-color: green;
}

Environment information:

  • less version: 4.2.2
  • nodejs version: any
  • operating system: any

We are currently in the middle of a battle of linters. We are told :not(a):not(b) is being classed as complex, and should be changed to :not(a, b)but it appears less forgets to expand when this feature is being used.4.2.2at least compiles the syntax (though incorrectly) our older version4.1.3` just keels over completely.. So I am guessing this is a relatively new feature and hasn't had much usage yet.

@WORMSS WORMSS added the bug label Jan 30, 2025
Copy link

dosubot bot commented Jan 30, 2025

The issue you're encountering with the & symbol not expanding within a selector list inside :not() is a known limitation in Less. In version 4.2.2, there was a fix related to nested pseudo-selector parsing, which might have improved some aspects of handling :not(), but it seems that the specific case of selector lists within :not() still doesn't expand the & symbol as expected [1].

This behavior indicates that Less currently does not support expanding the & symbol within a selector list inside :not(). The workaround would be to avoid using selector lists in :not() when you need the & symbol to expand, and instead use separate :not() pseudo-classes as you did in the .works example.

If this is causing issues with linters, you might need to adjust the linter rules or continue using the workaround until a future update potentially addresses this limitation.

To continue talking to Dosu, mention @dosu.


Help Dosu learn! Give it feedback: Great Response | Irrelevant Answer | Incorrect Sources | Too Verbose | Hallucination | Other

@WORMSS
Copy link
Author

WORMSS commented Jan 30, 2025

I am guessing @puckowski would be the best person for this type of issue? Due to his work on the latest fix for listed selectors?

@puckowski
Copy link
Contributor

Thank you for the issue report. I'll spend some time this weekend on this issue and I will try to come up with a fix PR.

@puckowski
Copy link
Contributor

I think I have a fix. Will work on adding more tests and try to get a PR submitted this weekend.

puckowski added a commit to puckowski/less.js that referenced this issue Jan 31, 2025
* Fix an issue where selector with comma would not expand correctly.
puckowski added a commit to puckowski/less.js that referenced this issue Feb 1, 2025
* Make fix for issue less#4313 selector list not expanding correctly more
  flexible.
puckowski added a commit to puckowski/less.js that referenced this issue Feb 1, 2025
* Add more tests for fix for issue less#4313.
puckowski added a commit to puckowski/less.js that referenced this issue Feb 1, 2025
* Cleanup merge issues for fix for issue less#4313.
@puckowski
Copy link
Contributor

PR #4314 should resolve this issue, but it looks like we may be having new CI problems.

@puckowski
Copy link
Contributor

FYI figured out the new CI issue so PR #4314 can be merged after review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants