chore(alpha): record master ancestry before alpha publish - #4506
Merged
Conversation
* Fix issue #4339 by limiting the whitespace check for the deprecation notice to not produce false positives.
* Correct deprecation notice for issue #4339.
* Fix for issue #4397 container query with variable names like @container @foo () {}.
fix:(issue#4397): container query variable names
Fix for issue #4339 false positive deprecation notice for mixins
…ess 5.x New deprecation infrastructure with automatic repetition limiting (max 5 per type): - deprecation.js: registry of deprecation IDs with descriptions - Parser warn() accepts deprecation IDs for categorized warnings - --quiet-deprecations: suppress only deprecation warnings (keeps other warnings) New deprecation warnings for features being removed in 5.x: - js-eval: inline JavaScript backtick expressions - at-plugin: @plugin directive Existing warnings now tagged with stable IDs: - mixin-call-no-parens, mixin-call-whitespace, dot-slash-operator - variable-in-unknown-value, property-in-unknown-value CLI deprecation notices for: --js, --line-numbers, --math=always
…t tracking
Results organized as:
results/latest/{system-id}.json - most recent per system
results/runs/{date}_{system-id}.json - historical archive (gitignored)
…nner
path.resolve('less') turns the package name into an absolute filesystem
path, preventing Node's package resolution from finding npm-installed
versions. Only resolve relative paths starting with '.'.
…iance_pct variance_pct was computing (max-min)/avg which is range-over-mean. Now uses stddev/avg (coefficient of variation) which is a proper variability statistic.
Prevents same-day runs from overwriting each other in the runs/ archive.
Deprecation warnings from flags like --js, --line-numbers, and --math=always were printed immediately during arg parsing, so --quiet-deprecations only worked if it appeared before the deprecated flag. Now all CLI deprecation messages are queued and flushed after parsing completes, respecting --silent, --quiet, and --quiet-deprecations regardless of flag order.
feat: deprecation system and benchmark suite for Less 5.x prep
Follows-up 53f84f0, which started the conditional with a check for `i + 1 < this.value.length`, which is the same as the parent block.
.substr() is deprecated so we replace it with .slice() which works similarily but isn't deprecated Signed-off-by: Tobias Speicher <rootcommander@gmail.com>
* Handle optional dependencies * Handle optional dependency image-size
* remove phantom stuff * lint fix * use deep clone
* Fix issue #4354 unknown at-rule expressions should not have commas in a keyword list. * Add some additional layer at-rule tests.
* Update README.md copyright year.
Co-authored-by: Timo Tijhof <krinkle@fastmail.com>
In Less.js 2.6.0, parsing of dimensions changed so that `5_large` is seen as one value, instead of as a list containing "5" and "_large". In updating the Less.php port, we forgot to consider this change because none of the Less.js 3.13 tests seem to cover this behavior. Follows-up #2485. This adds the test case from #2462, as inpired by downstream https://gerrit.wikimedia.org/r/1197310. Co-authored-by: Timo Tijhof <krinkle@fastmail.com>
…#4407) * fix(#4331): exclude CSS at-rule keywords from declarationCall parsing * fix(#4331): normalize spacing after CSS at-rule keywords in media queries When `and`, `or`, `not`, or `only` keywords appear without a space before `(` in media queries, ensure spacing is added in the output to produce valid CSS.
…#4409) * fix: correct import and error handling in style() function - Fix incorrect import: `Anonymous` was imported from '../tree/variable' instead of '../tree/anonymous' (worked by accident since Variable was imported on the line above) - Simplify switch/case with single case 0 to a plain if statement - Add explanatory comment to the catch block documenting why it exists (CSS pass-through for @container style() queries) * refactor: remove dead boolean logic in evalRoot() - Remove `allAmpersands` variable that was initialized to false and never set to true, making it dead code - Replace string-based ampersand detection (genCSS + regex) with direct element value checks, avoiding unnecessary AST-to-string conversion - Simplify boolean conditions that referenced the dead variable * fix: add missing parserInput.forget() in colorOperand The colorOperand parser rule called parserInput.save() but only called restore() on failure, missing the forget() call on the success path. * refactor: QueryInParens eval() returns new node instead of mutating this QueryInParens.eval() was mutating `this` directly instead of returning a new node, violating the core Less.js tree pattern. It also used a brittle queue pattern where deep copies were pushed to an `mvalues` array during eval() and shifted off during genCSS(). Now eval() creates and returns a new QueryInParens with evaluated children, and genCSS() reads directly from the node's properties. The `copy-anything` import is removed from this file (still used elsewhere in the codebase). * refactor: extract mergeRules into shared utility to fix AtRule layering violation AtRule.eval() was directly calling ToCSSVisitor.prototype._mergeRules, which breaks the architectural boundary between tree nodes and visitors. Extract the merge logic into a standalone utility (merge-rules.js) that both AtRule.eval() and ToCSSVisitor can use without coupling. * fix: remove Container copy-paste duplication and fix evalNested splice index bug Container was overriding evalNested, permute, and bubbleSelectors with identical copies of the methods already provided by NestableAtRulePrototype. Remove the redundant overrides so Container properly inherits from the shared prototype. Also fix a bug in NestableAtRulePrototype.evalNested where context.mediaBlocks.splice(i, 1) used `i` (the index into `path`) to splice `mediaBlocks`. These are different arrays with different contents, so the index was wrong. Use indexOf(this) to find the correct position.
* fix(benchmark): fix division in benchmark files for v4 math defaults Wrap bare divisions inside percentage() calls in extra parens so benchmarks work with v4's default parens-division math mode. Add --math option passthrough to benchmark-runner.js and pass --math=always in run-historical.sh for consistent cross-version results. * perf: remove unnecessary closures in hot paths - Remove `extendVisitor` alias in findMatch, use `this` directly - Replace IIFE closure for functionRegistry lookup in Ruleset.eval with inline loop ~5% improvement on main benchmark (median 38.6ms → 37.1ms) * perf: replace forEach/map closures with for loops in hot paths - Selector.eval: replace map() closures with pre-allocated for loops - Ruleset transformDeclaration: replace forEach with for loop - extend-visitor visitRuleset: replace forEach with for loop, cache extend and pathCount to reduce repeated property access Combined with previous commit: ~8% improvement on 104KB benchmark (median 38.6ms → 36.4ms) * fix(benchmark): handle all v3.12+/v4.x build scenarios - Use pnpm for v4.3+ (workspace: protocol) - Fallback tsc installation when npm can't install locally - Install runtime deps separately when npm fails due to unpublished workspace packages (@less/test-import-module) - Use last patch version of each minor release - Skip v3.13.x (broken source: missing tree/util.js) * bench: update benchmark results after hot-path optimizations Median: 39.07ms → 34.32ms (~12% improvement) Throughput: 2,495 KB/s → 2,828 KB/s System: macbook-pro arm64 * bench: add historical benchmark results and track runs in git - Add historical benchmark data (v3.5–v4.2) to results/runs/ - Update latest/ with all versions including v4.5.0-dev optimized results - Format JSON with 2-space indentation - Update .gitignore to track runs/ (historical records belong in git) * bench: full historical benchmark run (v2.0–v4.5, 23 versions) Apple M4 Pro, arm64, Node v18/v20/v24 Key findings: - v2.4-v2.5 fastest era (~31ms median on 104KB file) - v3.10-v3.12 massive regression (3-5x slower, 126-185ms) - v4.0 recovered to ~40ms - v4.2 fastest v4.x (35.4ms) - v4.5.1 current master: 42.2ms * bench: prune version list to significant performance changes Reduced from 23 to 15 versions based on full benchmark data. Dropped versions with <5% difference from their predecessor: - v2.1 (broken), v2.5, v2.7 (plateau with v2.4/v2.6) - v3.6–v3.9 (all within 1ms, flat ~41ms) - v4.1 (identical to v4.0) The full set can still be run with --versions flag.
* feat: migrate to native ESM with no build step - Rename src/ to lib/ — source files are shipped directly, no compilation - Add "type": "module" to package.json for native ESM support (Node 18+) - Convert bin/lessc, test files, and build scripts from CJS to ESM - Rename Gruntfile.js and .eslintrc.js to .cjs (must remain CommonJS) - Add .js extensions to all relative import paths for ESM resolution - Use createRequire() for optional dependency resolution (npm packages, JSON) - Configure TypeScript for check-only mode (noEmit: true, allowJs: true) - Update Rollup config to read from lib/ directly - Update CI matrix to drop Node 16 (minimum Node 18+) - Browser build is smaller: 500KB (was 509KB), minified 153KB (was 158KB) - All 139 tests pass * chore: fix trailing semicolons from linter * chore: gitignore generated .css.map files in lib/ * fix(ci): restore lts/-3 to test matrix * chore: stop tracking dist/ build artifacts Generated browser bundles don't need to be in source control — they're built during publish and included in the npm package via the files field. Removes duplicate copies from both root dist/ and packages/less/dist/. * fix(ci): use pnpm exec for playwright install npx doesn't reliably find binaries with pnpm. Since playwright is already a devDependency, use pnpm exec to run the installed version. * fix(ci): use pnpm --filter for playwright, disable fail-fast pnpm exec at workspace root can't find playwright binary since it's a devDependency of the less package. Use --filter to run in that context. Also disable fail-fast so all matrix jobs complete independently. * fix(ci): move playwright to root devDependencies Makes pnpm exec playwright work from workspace root in CI. * fix: upgrade copy-anything to v3 for ESM compat, fix Windows test paths copy-anything v2 lacks "type": "module", causing named import failures on Node 18. v3 has proper ESM exports. Revert testFolder to absolute path (matching original behavior) so debug test path replacements match Less compiler output on Windows. * chore: add CodeRabbit config to raise file review limit * fix: add files field to package.json, remove postinstall from published package Restricts npm package to only bin/, lib/, dist/, index.js, and README.md. Previously shipped test files, Gruntfile, eslint config, etc. Removes postinstall script (Playwright browser install) which only applies in the monorepo dev environment and fails when installed from npm. Verified: npm pack --dry-run shows 120 files (was 229), lessc CLI and API both work from a clean tarball install.
* refactor: convert prototype-based tree nodes to ES6 classes
Convert all 30 tree node files from `Object.assign(new Node(), {...})`
prototype pattern to proper `class extends Node` syntax. This enables
TypeScript to understand the inheritance chain, reducing checkJs errors
from 2756 to 0.
- All tree nodes now use `class X extends Node` (or appropriate parent)
- Node.type converted from instance property to getter for clean override
- Factory functions in index.js updated to use `new` instead of
Object.create + apply (required for ES6 class compatibility)
- Benchmark script converted to ESM
- Added @types/node devDependency for checkJs support
- Enabled checkJs in tsconfig.json
- Added JSDoc types to node.js base class and several utility files
No behavioral changes - all 139 tests pass, benchmark performance
unchanged vs historical baselines (avg 36-39ms for 104KB).
* fix: @plugin deprecation says "replaced" not "removed"
* fix: use constructor params for AtRule selectors, path.resolve in benchmark
* fix: align @types/node with engines.node >=18 floor
* feat: add JSDoc type annotations with @ts-check to all tree node files Add proper JSDoc type annotations to all 44 files in lib/less/tree/, enabling per-file TypeScript checking via @ts-check. No {*} or {any} casts — all types are derived from reading the actual code. Key changes: - Shared types (EvalContext, CSSOutput, TreeVisitor, FileInfo, VisibilityInfo) defined in node.js - Node.value typed as union: Node | Node[] | string | number | undefined - Node.prototype.parse declared for parser-injected prototype property - Constructor properties explicitly declared with proper types - Inline casts used to narrow union types at usage sites - Widened base class params where subclasses pass different types Also adds typecheck to prepublishOnly and pre-commit hook to catch regressions as more files are annotated toward global checkJs: true. All 139 tests pass, zero TypeScript errors. * fix: remove duplicate JSDoc type annotation in ruleset.js
* Fix color calc() inside from expression parsing issues. * Add tests for #4316. Co-authored-by: Matthew Dean <matthew-dean@users.noreply.github.com>
A nested @supports (or @document) builds an implicit, non-root ruleset that never gets a debugInfo attached during parsing. With dumpLineNumbers enabled, genCSS tried to read lineNumber/fileName off that missing debugInfo and threw a TypeError instead of producing output. Skip emitting debug info when a node has none, the same way nodes without a recorded line are already handled elsewhere.
* Preserve container feature function spacing Signed-off-by: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com> * Handle non-ASCII container names --------- Signed-off-by: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com> Co-authored-by: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Initial plan * Fix failing Request Copilot review CI job by handling 403 gracefully --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* feat: deprecate bare @variable in non-value at-rule positions Bare @var in at-rule preludes, names, and identifiers is deprecated in favour of @{foo} interpolation; the bare form still resolves, so this is a warning only (id: variable-in-at-rule-prelude, respects --quiet-deprecations and the repetition cap). Covered positions: - @media / @container feature preludes - @supports / @document / unknown & custom at-rule preludes - @Keyframes / @counter-style / @charset identifiers - @layer names and lists - @namespace prefix @{foo} interpolation is now accepted in these positions as the migration target (previously it errored in most of them). A bare @var in a nested declaration value -- e.g. @supports (display: @v) or @media (min-width: @v) -- is NOT deprecated: it is a declaration value and stays valid, detected via paren-depth awareness so parsing and output are unchanged. Value-position parsing is otherwise untouched: @var works, @{var} is not newly accepted in top-level declaration values. Migrates existing fixtures to @{var} and adds a dedicated fixture locking in backward-compatible resolution of the bare form. * fix: also deprecate @@variable-variable prefix in @namespace The @namespace prefix lookahead used `@[\w-]`, which misses an indirect `@@ref` (variable-variable) reference — entities.variable() accepts `@@name`, so `@namespace @@ref "..."` fell through to expression() and resolved without the deprecation warning. Widen the lookahead to `@@?[\w-]` so @@-prefixes hit the same warning path. Adds fixture coverage.
…#4461) * Fix #4460: parse comparison/range syntax in container style() queries The mediaFeature lookahead regex only matched a bare identifier before a comparison operator (=, >, <, >=, <=), so it failed whenever the operand was a function call, e.g. var(--n) or calc(6/2). Widened the regex to also match a single level of balanced parens before the operator. Added regression tests covering: @container style(var(--n) = 3) @container style(calc(6 / 2) = var(--n)) @container style(var(--size) > 1lh) * Refactor parser.js for improved readability --------- Co-authored-by: dweep <existing1.001@gmail>
* refactor: extract shared ESLint config and add lint scripts Addresses discussion #3787 by extracting shared ESLint rules to config/eslint/base.cjs and adding lint/lint:fix npm scripts. Changes: - Created config/eslint/base.cjs with common ESLint rules - Updated packages/less/.eslintrc.cjs to extend the shared config - Added lint and lint:fix scripts to root package.json The shared config maintains compatibility with both JS and TS files. TypeScript-specific recommended rules are scoped to .ts files only to avoid noise in legacy .js source files. Verified: pnpm run lint passes with zero errors, 139/139 unit tests pass (pre-commit hook failed only on unrelated port conflict). * style: apply eslint --fix formatting Auto-generated by 'pnpm run lint:fix' using the new shared config. Touches only quote style and indentation; no logic changes. - benchmark/benchmark-runner.js: indent - build/rollup.js: quotes (backtick -> single) - lib/less-node/environment.js: indent - lib/less/tree/nested-at-rule.js: indent * test(benchmark): add JSDoc for coverage Addresses docstring coverage warning in PR #4459 by adding full JSDoc to all functions in the benchmark-runner script.
* Replace image-size with probe-image-size * validation * Use loaded contents for image size probing * Use loaded contents for image size probing
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…bare-@var scan, add warnings coverage (#4469) * test: migrate remaining bare @variable at-rule fixtures to @{variable} Follow-up to #4462, which deprecated bare @variable in non-value at-rule positions and migrated most fixtures to @{variable} but left two feature fixtures on the bare form: - tests-unit/layer/layer.less @layer @layer-name - tests-unit/import/import/import-reference.less @Keyframes @keyframeName Migrated to @{layer-name} / @{keyframeName} (byte-identical render). Also repairs pnpm-lock.yaml: master had a dangling `minimatch: 3.1.2` dependency edge with no package entry (bad-merge artifact), so `pnpm install --frozen-lockfile` failed for every PR. Repinned to the resolved 3.1.5 already present; no dependency version changes. * test: assert deprecation/warning emission + suppress warnings in test output less.js asserts errors via tests-error/*.txt but had no coverage that warnings actually fire, so deprecation notices were unguarded (nothing would catch a regression that silently stopped emitting one). - Suppress warnings from normal test output (they are noise across the corpus); set LESS_TEST_SHOW_WARNINGS=1 to see them. - Add testWarnings() (run from index.js) which installs a capturing logger listener and asserts each render-reachable warning fires: variable-in-at-rule- prelude (incl. bar[@v] top-level -> warns and (x:@v) decl-value -> no warn), js-eval, mixin-call-whitespace, mixin-call-no-parens, variable-in-unknown-value, dot-slash-operator, complex-selector, extend-no-match, compress, at-plugin. Documented gaps (not render-reachable): property-in-unknown-value (a $prop ref resolves via the entity path before the permissive text scan), math-always and dumpLineNumbers (registered in deprecation.js but never emitted via warn()). * refactor(parser): fold at-rule prelude bare-@var detection into $parseUntil (DRY) The at-rule-prelude deprecation detected a top-level bare @var two ways: the permissiveValue entity loop (structural), plus a standalone hasTopLevelBareVariable() that RE-SCANNED the same text $parseUntil had already walked, with its own hand-rolled paren counter (and no string/comment handling). Fold that second scan into $parseUntil's single pass: it already skips strings/comments/ escapes and tracks brackets, so add an opt-in `detectBareVar` that records the first bare @var (not @{interp}) seen at PAREN depth 0 — [...]/{...} don't shield a reference, only a declaration-value (...) does — exposed as `.bareVarIndex`. $parseUntil has a single caller (permissiveValue), so the extra arg/property is contained. Delete hasTopLevelBareVariable. Behaviour preserved (regression-guarded by testWarnings): @foo @bar -> 1, @A and @b -> 2, bar[@v] -> 1 (bracket is top-level), (x:@v) -> 0 (decl value), and the mixed (a:@x) y[@z] -> 1. Also drops the testWarnings 'variable-in-unknown-value' case: it only fires for the inconsistent bracket edge (--x: bar[@bar]) while --x: @bar / 1px @bar / foo(@bar) resolve silently, so asserting it would lock in an artifact (now documented).
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…sions (#4472) * Initial plan * Fix boolean comparison of inline condition expressions --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
…charset (#4475) * test: migrate remaining bare @variable at-rule fixtures to @{variable} Follow-up to #4462, which deprecated bare @variable in non-value at-rule positions and migrated most fixtures to @{variable} but left two feature fixtures on the bare form: - tests-unit/layer/layer.less @layer @layer-name - tests-unit/import/import/import-reference.less @Keyframes @keyframeName Migrated to @{layer-name} / @{keyframeName} (byte-identical render). Also repairs pnpm-lock.yaml: master had a dangling `minimatch: 3.1.2` dependency edge with no package entry (bad-merge artifact), so `pnpm install --frozen-lockfile` failed for every PR. Repinned to the resolved 3.1.5 already present; no dependency version changes. * test: assert deprecation/warning emission + suppress warnings in test output less.js asserts errors via tests-error/*.txt but had no coverage that warnings actually fire, so deprecation notices were unguarded (nothing would catch a regression that silently stopped emitting one). - Suppress warnings from normal test output (they are noise across the corpus); set LESS_TEST_SHOW_WARNINGS=1 to see them. - Add testWarnings() (run from index.js) which installs a capturing logger listener and asserts each render-reachable warning fires: variable-in-at-rule- prelude (incl. bar[@v] top-level -> warns and (x:@v) decl-value -> no warn), js-eval, mixin-call-whitespace, mixin-call-no-parens, variable-in-unknown-value, dot-slash-operator, complex-selector, extend-no-match, compress, at-plugin. Documented gaps (not render-reachable): property-in-unknown-value (a $prop ref resolves via the entity path before the permissive text scan), math-always and dumpLineNumbers (registered in deprecation.js but never emitted via warn()). * refactor(parser): fold at-rule prelude bare-@var detection into $parseUntil (DRY) The at-rule-prelude deprecation detected a top-level bare @var two ways: the permissiveValue entity loop (structural), plus a standalone hasTopLevelBareVariable() that RE-SCANNED the same text $parseUntil had already walked, with its own hand-rolled paren counter (and no string/comment handling). Fold that second scan into $parseUntil's single pass: it already skips strings/comments/ escapes and tracks brackets, so add an opt-in `detectBareVar` that records the first bare @var (not @{interp}) seen at PAREN depth 0 — [...]/{...} don't shield a reference, only a declaration-value (...) does — exposed as `.bareVarIndex`. $parseUntil has a single caller (permissiveValue), so the extra arg/property is contained. Delete hasTopLevelBareVariable. Behaviour preserved (regression-guarded by testWarnings): @foo @bar -> 1, @A and @b -> 2, bar[@v] -> 1 (bracket is top-level), (x:@v) -> 0 (decl value), and the mixed (a:@x) y[@z] -> 1. Also drops the testWarnings 'variable-in-unknown-value' case: it only fires for the inconsistent bracket edge (--x: bar[@bar]) while --x: @bar / 1px @bar / foo(@bar) resolve silently, so asserting it would lock in an artifact (now documented). * deprecate dash-only variable names * deprecate dynamic charset interpolation * chore: release v4.8.0
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Matthew Dean <matthew-dean@users.noreply.github.com>
…#4477) Signed-off-by: 林晨 (Leo Cheng) <leo-cheng@vip.qq.com>
…me CSS var() (#4479) Signed-off-by: 林晨 (Leo Cheng) <leo-cheng@vip.qq.com>
* fix(release): sync release version from PR title * fix(release): harden title sync automation * fix(release): harden title sync workflow * fix(release): make changelog title sync idempotent * fix(release): insert missing changelog heading on title sync * fix(release): insert changelog heading without prior releases
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* feat: support `[...]` lookups in `@{...}` interpolation
Backports the Less 5.x behaviour of allowing a lookup chain inside variable
interpolation, so `@{map[key]}` resolves wherever `@{name}` already does:
selectors, property names, at-rule preludes and names, quoted strings, `url()`
and `@import` paths. Chained (`@{map[a][b]}`), variable (`@{map[@k]}`),
indirect (`@{map[@@k]}`) and empty (`@{map[]}`) keys are all supported.
Previously these either failed to parse or — inside strings and `url()` —
were emitted verbatim into the output with no error and no warning.
The parser does not gain a lookup regex: `entities.variableCurly` now matches
`@{` plus the name and delegates the chain to `parsers.mixin.ruleLookups()`,
the same production the bare `@map[key]` form uses. Only the two eval-time
string paths (`Quoted`, inline JS) need a pattern, and they share one
definition rather than three hand-maintained copies — that duplication is what
let `quoted.js` drift from `lookupValue` and cause the silent pass-through.
`${...}` is deliberately left narrow. Properties have no lookup grammar
(`entities.property` parses `$name` with no chaining) and nothing can hold a
ruleset to look into, since `prop: { ... }` is a parse error in every scope.
Also fixes three related defects:
* A bare lookup in a structural position was exempt from the
`variable-in-at-rule-prelude` deprecation. `entities.variable()` parses
`@map[key]` into a `NamespaceValue`, and both warning sites tested only for
`Variable`, so `@keyframes @Map[key]`, `@supports @Map[key]` and
`@layer @Map[key]` warned for a plain `@var` but stayed silent for a lookup.
A lookup in a value position — `@supports (width: @Map[key])` — correctly
remains undeprecated.
* `@supports (width: @Map[key])` rendered as `(width: [key])`, dropping the
variable. Unknown at-rule preludes are scanned as text, so a bare lookup
reached the permissive regex, which matched only `@map` and left `[key]`
behind. The resulting condition never parsed, so the block was dead in every
browser; it now resolves.
* Interpolation inside an unquoted `url(@{path}/x.png)` was never substituted.
The body was raw text in an `Anonymous` node; text containing interpolation
is now handed to an escaped `Quoted`, matching the quoted spelling.
The `variable-in-unknown-value` notice is now tested against the text with
interpolations stripped, so a variable key inside `@{map[@key]}` is no longer
misreported as a bare use of the syntax the notice recommends adopting.
* fix: preserve unquoted url escaping and keep `${...}` lookup-free
Addresses two review findings.
`URL.eval` escapes a rewritten rootpath only when the value is unquoted, so
wrapping an interpolated unquoted `url()` body in a `Quoted` carrying a real
quote character suppressed that escaping. With a rootpath containing `(`, `)`
or whitespace it emitted `url(a(b)/x.png)` — malformed — where the
non-interpolated spelling correctly produced `url(a\(b\)/x.png)`.
The body is now built with an empty quote string, which reads as unquoted while
still resolving interpolation. That alone was not enough: `Quoted.eval`
rebuilt its result as `this.quote + value + this.quote` and re-derived the
quote from that string, so an empty quote picked up the first character of the
substituted value and read as quoted again. The quote is now carried across
explicitly — a no-op for real quote characters, where the two already agree.
`RULE_PROPERTY_PARTICLE` shared `[@$]` across both sigils, so `${name[key]}`
was accepted as a property-name particle and reached
`resolveInterpolatedProperty` with `name[key]`, producing a misleading
undefined-property error for syntax the language does not define. The two
sigils are now spelled out separately: lookup chains for `@{...}` only.
Adds `tests-config/rootpath-escape-interpolation`, whose rootpath deliberately
contains `(`, `)` and a space so quoted and unquoted forms cannot look alike,
covering plain, interpolated and lookup bodies in both spellings.
* test: run the error fixtures again
The glob branch of `runTestSetInternal` admitted a `.less` file only when a
sibling `.css` existed. Error fixtures declare their expectation as a `.txt`
and never produce CSS, so every one of them was silently skipped — the file was
globbed, failed the existence check, and was dropped without being counted.
That covered 96 fixtures: 27 in tests-error/parse, 67 in tests-error/eval, and
the js-type-errors and no-js-errors sets. The suite reported 136 passing tests
and stayed green even when an expectation was edited to something impossible,
so parse and eval error regressions went uncaught entirely.
The check now accepts either expectation form. Verified both directions:
breaking an expectation now fails the run (exit 1 with a named FAIL), where
before it passed silently.
Re-enabling the suite surfaced two stale expectations, both pre-existing drift
rather than behaviour changes — `property-undefined` and `recursive-property`
carried a trailing blank line that `err.toString()` does not produce. Stock
4.8.1 emits the same text as this branch for both, confirming the fixtures had
drifted while dormant. Their trailing newline is corrected here.
Also adds the parse fixture for `${name[key]}`, which is now enforceable.
Test count goes from 136 to 233.
* ci: test the declared node floor explicitly
`packages/less` declares `engines.node: >=18`, but the oldest CI job tracked
`lts/-3`. A relative selector drifts upward as new LTS lines ship, so the
declared minimum stops being exercised the moment another line reaches LTS —
silently, since nothing ties the matrix to the engines field.
Pins that job to `18` so the supported floor is actually tested.
* test: drop the pre-Node-16.9 type-error fixture
`testTypeErrors` selected between two expectations with
`semver.gte(process.version, 'v16.9.0')`. The two differ only in V8's wording
change at 16.9 — `Cannot read property 'x' of undefined` became
`Cannot read properties of undefined (reading 'x')` — so with
`engines.node: >=18` the older `.txt` was unreachable on every supported
runtime.
The `-2` variant becomes the only expectation and the branch goes away. That
leaves `semver` unused, so it is dropped from devDependencies too.
Verified the fixture is genuinely enforced after the rename: editing its
expectation fails the run, where the whole set was skipped entirely before the
preceding commit re-enabled it.
* fix: offset the synthetic url Quoted by currentIndex, and cover url mappings
`entities.quoted()` and the `URL` node both build with `index + currentIndex`,
but the synthetic `Quoted` for an interpolated unquoted body stored only the
local `index`. Aligned for consistency.
This is currently unobservable: `currentIndex` is non-zero only inside
`parseNode`, and none of its three callers can reach `entities.url()` — two
parse `['selector']`/`['selectors']`, and the third re-parses declaration
values that were stored as `Anonymous`, which `anonymousValue` cannot produce
for text containing `(`. The offset is correct regardless, and stops the node
from being the odd one out if another caller appears.
Adds a sourcemap fixture for url() values, which is the part that could have
regressed: interpolated unquoted bodies now build a `Quoted` where they used to
build an `Anonymous`, and the two differ in `genCSS` — `Anonymous` passes
fileInfo and index to `output.add`, an escaped `Quoted` does not. Measured
before adding it, the mapping structure is unchanged (an escaped `Quoted`
contributes no segment of its own, and the enclosing declaration already
carries the position). The fixture pins that, covering literal, interpolated,
lookup and quoted-interpolated bodies side by side; the harness validates all
four mappings against source.
* test: run the sourcemap fixtures again
`sourcemaps/basic` and `sourcemaps/custom-props` were skipped for the same
reason the error fixtures were: the glob branch required a sibling `.css`, and
these keep their expectation in `test/sourcemaps/*.json` via the set's
`getFilename`. Neither had run since that gate was introduced.
Generalises the previous fix rather than extending it — a fixture opts in by
declaring an expectation in any of the three supported ways: a sibling `.css`
for the default compile-and-diff, a sibling `.txt` for the error sets, or a
`getFilename` that resolves one elsewhere.
Both fixtures then failed on stale metadata, with mappings byte-identical:
- Their config had drifted to `sourceMap: true`. The harness only fills in
`sourceMapRootpath`/`sourceMapOutputFilename` when `sourceMap` is an object,
so the `testweb/` prefix the expectations were written against disappeared.
Restored to `{}`, which also matches the sibling sourcemaps-* sets.
- The expectations still carried pre-monorepo paths, missing the `tests-config/`
segment added when fixtures moved to `packages/test-data`. Regenerated; only
`sources` and `file` change, the mappings are unchanged.
`basic` validates 52 mappings and `custom-props` 1.
Two fixtures remain skipped and are left alone: sourcemaps-disable-annotation
and sourcemaps-variable-selector read `test/<name>.json`, a path layout that no
longer exists, and both are named `basic.less`, so they would collide under the
`test/sourcemaps/` convention. Fixing them needs a rename.
* test: run the remaining sourcemap fixtures again
Correcting the previous commit's note: the expectations for
sourcemaps-disable-annotation and sourcemaps-variable-selector were not missing,
and no rename was needed. They sit in their own directories under
`packages/less/test/`, so there was never a collision — I had looked in
`test/*.json` and `test/tests-config/` but not `test/sourcemaps-*/`.
The real fault was path resolution. Both verifiers read
`path.join('test/', name)`, but fixture names gained a leading `tests-config/`
when the suite moved to packages/test-data, while the expectations stayed put.
Resolution now strips that segment via a shared helper.
With the paths fixed, the gate no longer needs to special-case them: a set with
its own verifyFunction is trusted to locate and report its own expectation, so
the `.css` requirement applies only to the default compile-and-diff.
Both then failed on the same stale metadata as basic/custom-props, mappings
byte-identical: `sourcemaps-variable-selector` had drifted to `sourceMap: true`,
which skips the harness's `testweb/` defaults, and both expectations carried
pre-monorepo paths. Config restored to `{}` and expectations regenerated —
`sources` and `file` change, mappings do not.
This also picks up the sourcemaps-variable-selector `vars` fixture.
* fix(test): resolve sourcemap expectations on Windows
`name` is assembled from `path.relative`, so its separators are platform
native. Stripping the `tests-config/` prefix with a forward-slash-only pattern
left it in place on Windows, and the two fixtures looked for an expectation
under a path that has never existed — reported as an empty expected value
rather than a missing file.
Separators are normalised before the prefix is stripped. Verified against both
spellings, including the mixed form `path.relative` actually produces there
(`tests-config\set/basic`).
Co-authored-by: snowyukitty <270071858+snowyukitty@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
dosubot
Bot
added
the
size:XS
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
masteras merged intoalphausing an ours-strategy merge5.0.0-alpha.1manifests unchangedalphacontains all commits frommasterWhy
The alpha publish for #4503 failed before npm because
publish.ymldetected thatalphawas behindmaster. This PR updates branch ancestry without pulling the Less 4 implementation tree into the Less 5 rewrite.Verification
HEAD..upstream/masteris 0, current version is5.0.0-alpha.1, master version is4.9.0pnpm run test:releasegit diff --checkAfter this merges, rerun the failed #4503
Publish to NPMworkflow or let the release automation create a fresh alpha release PR from the now-current alpha branch.