Repository navigation
Conversation
bin/lessc is an ESM file in a type:module package; without an extension Node raises ERR_UNKNOWN_FILE_EXTENSION on runtimes without default module-syntax detection (before v22.7.0). Renames it to bin/lessc.js and updates the bin mapping, the 13 Gruntfile shell invocations and one comment. Resolves less#4561.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Option parsing and plugin loading packages/less/bin/lessc.js |
The CLI parses compiler options, queues deprecation warnings, and loads queued plugins before rendering. |
Input rendering and output packages/less/bin/lessc.js |
The CLI validates render settings, reads Less input or stdin, renders it, and handles CSS, source-map, and dependency output. |
Executable path updates packages/less/package.json, packages/less/Gruntfile.cjs, packages/less/lib/less/parse-tree.js |
The package binary mapping and Grunt commands use lessc.js. A source-map comment identifies the new path. |
Priority:
Estimated code review effort: 4 (Complex) | ~45 minutes
Change: Bug fix · Severity of issue fixed: Medium
Suggested reviewers: matthew-dean
Merge Risk: 
32c4b
-
The executable path updates are consistent, and no actionable regression remains identified. This change is mergeable after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "fix: give the lessc bin a .js extension" matches the main change: the bin file is renamed from bin/lessc to bin/lessc.js, and the package.json bin mapping, 13 Gruntfile shell invocations, a… Linked Issues check Issue #4561has one active coding requirement: make thelesscCLI start under"type": "module"by giving the bin an unambiguous ESM extension, and keepnpx lessc myFile.less myFile.cssworking. …Out of Scope Changes check All changed files connect directly to the #4561objective:
-packages/less/package.json: bin mapping to./bin/lessc.js(required fix).
-packages/less/Gruntfile.cjs: 13 shell invocations changed…Full details: Title check
-
Explanation
The title "fix: give the lessc bin a .js extension" matches the main change: the bin file is renamed from bin/lessc to bin/lessc.js, and the package.json bin mapping, 13 Gruntfile shell invocations, and one comment are updated to match. The linked issue
#4561reports the ERR_UNKNOWN_FILE_EXTENSION failure that this rename addresses. The title is short, specific, and states the primary change from the developer's perspective.
Full details: Linked Issues check
-
Explanation
Issue
#4561has one active coding requirement: make thelesscCLI start under"type": "module"by giving the bin an unambiguous ESM extension, and keepnpx lessc myFile.less myFile.cssworking. The PR meets this requirement:packages/less/package.jsonnow mapslesscto./bin/lessc.js(confirmed by diff against the merge base). The.jsextension makes Node load the file as ESM, which removes theERR_UNKNOWN_FILE_EXTENSIONfailure.packages/less/bin/lessc.jsexists at the reviewed head with the same ESM source and hashbang as the original bin file.packages/less/Gruntfile.cjsinvokesbin/lessc.jsor../bin/lessc.jsin theshell:opts,shell:plugin, andshell:sourcemap-testtasks (13 invocations, confirmed by diff). Automated CLI coverage therefore exercises the new path.- The issue's suggested fix (rename to
.jsand updatebin) is implemented. The issue does not require new tests. The author reportsnpm run test:nodepassing (281 tests). I did not run tests, so that result is author-reported and unverified here.
No unmet coding requirement was found.
Full details: Out of Scope Changes check
-
Explanation
All changed files connect directly to the
#4561objective:packages/less/package.json: bin mapping to./bin/lessc.js(required fix).packages/less/Gruntfile.cjs: 13 shell invocations changed frombin/lessctobin/lessc.jsso existing tasks run the renamed entry point (necessary consequence of the rename).packages/less/bin/lessc.js: the renamed CLI file (required fix).packages/less/lib/less/parse-tree.js: comment-only update referencing the new path; no runtime change (related, non-functional).
Non-blocking observation: at the reviewed head, the original extensionlesspackages/less/bin/lesscis still tracked alongsidebin/lessc.js. The PR description says the file was moved withgit mv, but the repository shows a copy. The leftover file is not referenced bypackage.jsonor the Gruntfile. The issue explicitly allows keeping an extensionless shim, so this does not count as an unrelated change. Removing the leftover file would avoid a duplicate in the publishedbinfolder, but this is not a scope failure.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
 [Medium impact] The change affects users who launch Less through its command-line compiler. The PR appears safe to merge, with a non-blocking cleanup needed for the duplicate CLI.
|
| import path from 'path'; | ||
| import os from 'os'; | ||
| import { createRequire } from 'module'; | ||
| import fs from '../lib/less-node/fs.js'; | ||
| import * as utils from '../lib/less/utils.js'; | ||
| import * as Constants from '../lib/less/constants.js'; | ||
| import less from '../lib/less-node/index.js'; |
There was a problem hiding this comment.
The new lessc.js copies all 678 lines of bin/lessc, but the old file remains tracked and both files ship in the package. Future fixes could update one copy while leaving the other behind. Complete the rename by removing bin/lessc, or make it a small wrapper if the old path must remain supported.
The previous commit added bin/lessc.js but kept the original bin/lessc, so both shipped as byte-identical copies. Remove the old file so the change is the rename the PR describes.
Resolves #4561
Root cause
Since 4.6.0 the
lesspackage declares"type": "module"and itsbinentry pointsat the extensionless
bin/lessc. When Node loads an extensionless file in an ESMcontext it raises:
Node's module-syntax detection (
--experimental-detect-module, default sincev22.7.0) rescues newer runtimes — which is why the reporter found that "upgrading my
node version fixes this" — but on Node releases without it (e.g. 20.x and 22.0–22.6,
which include current LTS lines) the CLI cannot start at all. The extensionless bin
is the packaging bug: the loader has no way to know the file is ESM.
Fix
Give the bin a proper extension so every Node version loads it unambiguously:
bin/lessc→bin/lessc.js(git mv; contents unchanged — the file is already ESMwith a hashbang, which Node supports in
.jsfiles)packages/less/package.json:"bin": { "lessc": "./bin/lessc.js" }packages/less/Gruntfile.cjs: the 13node bin/lessc …shell invocations updatedparse-tree.jsreferencing the old pathThe
filesfield already ships the wholebinfolder, so packaging is unaffected.The npm shim scripts are generated from the
binmapping, sonpx lessc/node_modules/.bin/lessckeep working with no user-visible rename.Validation
node packages/less/bin/lessc.js --version→lessc 4.9.1 (Less Compiler)npm run test:node→ 281 run, all passed — including theshell:optsandshell:plugingrunt tasks that invoke the renamed path 13 times, and the CJS buildtests
eslintclean on the touched files (the repo's enforced lint gate; noteprettier --checkflags these three files identically on unpatchedmaster, so noformatting regression is introduced)
Summary by CodeRabbit
lessccommand and related test commands now use the correct executable path.