less.js icon indicating copy to clipboard operation
less.js copied to clipboard

fix(issue:4331) and keyword treated as function

Open puckowski opened this issue 9 months ago • 6 comments

What:

Fix for issue #4331

The and keyword was being treated as a function call starting Less 4.2.1 which introduced selector generation problems.

Why:

The following Less:

@media screen and(max-width:1280px) {
  .form-process-table {
    width: 1200px;
    > div {
      width: 960px;
    }
  }
}

would fail to insert a space after and Keyword like it did in Less 4.2.0. This would cause selector issues in the resulting CSS.

I simultaneously had to add a function definition for size to avoid regressions with container queries when fixing this and issue.

Checklist:

  • [ ] Documentation
  • [x] Added/updated unit tests
  • [x] Code complete

Workaround: Ensure a space is present after and in the source Less.

puckowski avatar Mar 27 '25 22:03 puckowski

Looks like there are CI issues (may be resolved by https://github.com/less/less.js/pull/4322):

╔═════════════════════════════════════════════════════════════════════════╗
║ Looks like Playwright Test or Playwright was just installed or updated. ║
║ Please run the following command to download new browsers:              ║
║                                                                         ║
║     pnpm exec playwright install                                        ║
║                                                                         ║
║ <3 Playwright Team                                                      ║
╚═════════════════════════════════════════════════════════════════════════╝

puckowski avatar Mar 27 '25 22:03 puckowski

What change made it be treated like a function?

matthew-dean avatar Mar 30 '25 03:03 matthew-dean

What change made it be treated like a function?

https://github.com/less/less.js/pull/4237

The addition of declarationCall which must match functions before keywords (otherwise functions may be identified incorrectly as keywords) caused the regression:

e = entities.declarationCall.bind(this)() || entities.keyword() || entities.variable() || entities.mixinLookup();

#4237 was potentially relying on a weak regex:

validCall = parserInput.$re(/^[\w]+\(/);

though instead of fiddling with a regex in this PR we now check the function registry to sure what we are looking at is a function, otherwise we can later try to parse as keyword.

puckowski avatar Mar 30 '25 10:03 puckowski

@puckowski Seems like I'm getting Playwright errors even after merging the other one 🤔

matthew-dean avatar Mar 31 '25 19:03 matthew-dean

I pulled latest to get CI changes from https://github.com/less/less.js/pull/4333 and pushed again. CI is good now. I suspect any old PRs running into this issue will have to pull latest master and repush or rebase.

I suspect old pnpm-lock.yaml files in these older branches is what caused the failures when you reran.

@matthew-dean

puckowski avatar Mar 31 '25 20:03 puckowski

What do you think about merging this PR?

I'd like to get this fix out the door soon to resolve the regression. Subsequent PRs can improve the solution.

@matthew-dean

puckowski avatar Apr 19 '25 11:04 puckowski