fix: avoid TypeError on nested @supports with dumpLineNumbers#4446
Conversation
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.
📝 WalkthroughWalkthroughThe PR modifies the Less compiler's debug-info module to safely skip output generation when debug metadata is unavailable, and adds comprehensive test fixtures that verify Changes
🎯 2 (Simple) | ⏱️ ~12 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/less/lib/less/tree/debug-info.js (1)
10-12: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUpdate the typedef to reflect that
debugInfois optional.The
DebugInfoContexttypedef declaresdebugInfoas a required property, but the new guard on Line 64 allowsctx.debugInfoto beundefined. This creates a type mismatch between the signature and the implementation.📝 Proposed fix to make debugInfo optional in the typedef
/** * `@typedef` {object} DebugInfoContext - * `@property` {DebugInfoData} debugInfo + * `@property` {DebugInfoData} [debugInfo] */🤖 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/lib/less/tree/debug-info.js` around lines 10 - 12, The typedef for DebugInfoContext currently declares debugInfo as required but the code (see guard using ctx.debugInfo in debug-info.js) permits it to be undefined; update the JSDoc typedef to mark debugInfo optional (e.g., change the `@property` to use the optional syntax like [debugInfo] or DebugInfoData=) so the type matches the implementation and references to DebugInfoContext/documentation reflect that ctx.debugInfo may be absent.
🧹 Nitpick comments (1)
packages/test-data/tests-config/debug/linenumbers.less (1)
35-42: ⚡ Quick winConsider adding a test case for nested
@documentas well.The PR description mentions that both
@supportsand@documentare affected by this issue. While the fix should handle both, adding an explicit test case for@documentwould provide more comprehensive coverage.📋 Example test case for `@document`
.document-rule { `@document` url(https://example.com/) { color: purple; .nested-document { color: orange; } } }🤖 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-config/debug/linenumbers.less` around lines 35 - 42, Add a parallel test case covering nested `@document` rules: create a new block similar to .supports-rule but using `@document` (use the selector .document-rule and nested .nested-document) containing a top-level declaration and a nested declaration to mirror the structure in the diff; ensure the new test sits alongside the existing .supports-rule test so the parser/transformer behavior for `@document` is exercised the same way as for `@supports`.
🤖 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.
Outside diff comments:
In `@packages/less/lib/less/tree/debug-info.js`:
- Around line 10-12: The typedef for DebugInfoContext currently declares
debugInfo as required but the code (see guard using ctx.debugInfo in
debug-info.js) permits it to be undefined; update the JSDoc typedef to mark
debugInfo optional (e.g., change the `@property` to use the optional syntax like
[debugInfo] or DebugInfoData=) so the type matches the implementation and
references to DebugInfoContext/documentation reflect that ctx.debugInfo may be
absent.
---
Nitpick comments:
In `@packages/test-data/tests-config/debug/linenumbers.less`:
- Around line 35-42: Add a parallel test case covering nested `@document` rules:
create a new block similar to .supports-rule but using `@document` (use the
selector .document-rule and nested .nested-document) containing a top-level
declaration and a nested declaration to mirror the structure in the diff; ensure
the new test sits alongside the existing .supports-rule test so the
parser/transformer behavior for `@document` is exercised the same way as for
`@supports`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a506d09b-39ee-4881-ad55-a75fdeb7a6d2
📒 Files selected for processing (5)
packages/less/lib/less/tree/debug-info.jspackages/test-data/tests-config/debug/all/linenumbers-all.csspackages/test-data/tests-config/debug/comments/linenumbers-comments.csspackages/test-data/tests-config/debug/linenumbers.lesspackages/test-data/tests-config/debug/mediaquery/linenumbers-mediaquery.css
What:
Rendering valid Less with
dumpLineNumbersenabled throws aTypeErrorwhen a@supports(or@document) block is nested inside a selector, e.g.It happens in all three modes (
comments,mediaquery,all). Top-level@supports, and@media/@container, are unaffected. This is the long-standing #3073.Why:
A nested
@supports/@documentends up as an implicit, non-root ruleset that never gets adebugInfoattached during parsing (unlike@media, which threads one through). When that ruleset renders,debug-info.jsreadslineNumber/fileNameoff the missingdebugInfoand throws instead of producing CSS.The fix just skips emitting debug info for a node that has none — the same outcome you already get for any node without a recorded line, so nodes that do have line info are unchanged.
I added a nested
@supportsblock to the sharedlinenumbers.lessfixture; it reproduces the throw across the comments/mediaquery/all expectations before the change and passes after.grunt test:nodeis green.Checklist:
Summary by CodeRabbit
Bug Fixes
Tests
@supportsconditional feature with enhanced line number mapping across multiple debug output formats.