Feature/inequality operator#4454
Conversation
dosubot
Bot
added
the
size:L
📝 WalkthroughWalkthroughAdds Changes
Benchmark runner refactor and rollup cosmetic fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/test-data/tests-unit/css-guards/css-guards.less (1)
104-115: ⚡ Quick winAdd one fixture that covers non-comparable inequality.
The new cases validate comparable
!=paths, but they don’t exercise the non-comparable branch that should still evaluate!=as true. Add one guard likewhen (foo != 1)and assert it emits CSS.Suggested fixture addition
.inequality-test-4 when (`@b` != 2) { color: red; } + +.inequality-test-5 when (foo != 1) { + color: green; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/test-data/tests-unit/css-guards/css-guards.less` around lines 104 - 115, Add a new test fixture after the existing inequality-test cases that validates non-comparable inequality evaluation. Create a guard like when(foo != 1) where foo is an undefined or non-comparable variable, and include a CSS rule (such as color: green;) that should be emitted when the guard evaluates to true. This test should demonstrate that the non-comparable branch in the inequality operator evaluation correctly returns true when comparing incompatible types or undefined values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/less/benchmark/benchmark-runner.js`:
- Around line 46-47: The compiler discovery condition at line 46 currently uses
OR logic to accept compilers that have either `render` or `parse`, but the
`runOnce()` function always calls `less.render` at line 93. Change the condition
from `(less.render || less.parse)` to `(less.render && less.parse)` to require
both methods to be present, preventing runtime crashes when a parse-only
compiler is selected.
---
Nitpick comments:
In `@packages/test-data/tests-unit/css-guards/css-guards.less`:
- Around line 104-115: Add a new test fixture after the existing inequality-test
cases that validates non-comparable inequality evaluation. Create a guard like
when(foo != 1) where foo is an undefined or non-comparable variable, and include
a CSS rule (such as color: green;) that should be emitted when the guard
evaluates to true. This test should demonstrate that the non-comparable branch
in the inequality operator evaluation correctly returns true when comparing
incompatible types or undefined values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 01e6e179-62bd-4278-9565-48b010ac24d4
📒 Files selected for processing (6)
packages/less/benchmark/benchmark-runner.jspackages/less/build/rollup.jspackages/less/lib/less/parser/parser.jspackages/less/lib/less/tree/condition.jspackages/test-data/tests-unit/css-guards/css-guards.csspackages/test-data/tests-unit/css-guards/css-guards.less
| if (less && (less.render || less.parse)) { | ||
| lessPath = p; |
There was a problem hiding this comment.
Require render during compiler discovery to prevent runtime crashes.
At Line 46, discovery currently accepts compilers that only expose parse. But runOnce() always calls less.render (Line 93), so a parse-only module will fail at runtime with TypeError: less.render is not a function.
Proposed fix
- if (less && (less.render || less.parse)) {
+ if (less && typeof less.render === 'function') {
lessPath = p;
break;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (less && (less.render || less.parse)) { | |
| lessPath = p; | |
| if (less && typeof less.render === 'function') { | |
| lessPath = p; | |
| break; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/less/benchmark/benchmark-runner.js` around lines 46 - 47, The
compiler discovery condition at line 46 currently uses OR logic to accept
compilers that have either `render` or `parse`, but the `runOnce()` function
always calls `less.render` at line 93. Change the condition from `(less.render
|| less.parse)` to `(less.render && less.parse)` to require both methods to be
present, preventing runtime crashes when a parse-only compiler is selected.
1 participant
What:
!=) inpackages/less/lib/less/parser/parser.js.packages/less/lib/less/tree/condition.js:truewhen comparison results in inequality (-1or1).falsewhen comparison results in equality (0).truewhen comparison is undefined (types are not comparable, meaning they are not equal).packages/test-data/tests-unit/css-guards/css-guards.lessandcss-guards.css.Why:
Currently, Less lacks an inequality comparison operator, requiring users to write verbose guard checks like
not (@foo = 1)rather than the cleaner and standard@foo != 1. This aligns Less with standard CSS/JS and other CSS preprocessors.Checklist:
Summary by CodeRabbit
New Features
!=(not-equal) operator in guard conditions.Improvements
Tests