Skip to content

fix: handle multiline comments in stripComments#344

Open
mvtandas wants to merge 1 commit into
unjs:mainfrom
mvtandas:fix/multiline-comment-strip
Open

fix: handle multiline comments in stripComments#344
mvtandas wants to merge 1 commit into
unjs:mainfrom
mvtandas:fix/multiline-comment-strip

Conversation

@mvtandas

@mvtandas mvtandas commented Apr 14, 2026

Copy link
Copy Markdown

Summary

Fixes #279

Multiline block comments were not being stripped because the regex used .+? which doesn't match newline characters.

Reproduction

hasESMSyntax(`/* export * */`, { stripComments: true })     // false ✅
hasESMSyntax(`/* \n  export * \n*/`, { stripComments: true }) // true ❌ (should be false)

Fix

- const COMMENT_RE = /\/\*.+?\*\/|\/\/.*(?=[nr])/g;
+ const COMMENT_RE = /\/\*[\s\S]*?\*\/|\/\/.*(?=[nr])/g;

.+?[\s\S]*? matches any character including newlines (non-greedy).

Tests

Added 3 test cases for multiline block comments. All 201 tests pass.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed multi-line comment handling during syntax detection when the stripComments option is enabled, improving accuracy of module syntax classification.
  • Tests

    • Added test cases to validate multi-line comment handling in syntax detection.

The COMMENT_RE regex used `.+?` which does not match newline
characters, causing multiline block comments to not be stripped.

Changed to `[\s\S]*?` which matches any character including newlines.

Before: `/* \n export * \n*/` was not stripped → false ESM detection
After:  `/* \n export * \n*/` is correctly stripped

Fixes unjs#279
@coderabbitai

coderabbitai Bot commented Apr 14, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1bf96b0c-f49b-4353-a239-aa700077edba

📥 Commits

Reviewing files that changed from the base of the PR and between c5ce4e5 and c294f6d.

📒 Files selected for processing (2)
  • src/syntax.ts
  • test/syntax.test.ts

📝 Walkthrough

Walkthrough

Updated the block-comment stripping regex in syntax detection functions to properly match /* ... */ across newlines using [\s\S]*? instead of .+?. This resolves incorrect syntax detection results when stripComments is enabled on multiline commented-out code.

Changes

Cohort / File(s) Summary
Comment Stripping Regex Fix
src/syntax.ts
Modified the block-comment regex pattern from .+? to [\s\S]*? to correctly match multiline /* ... */ comments before syntax detection, affecting hasESMSyntax, hasCJSSyntax, and detectSyntax functions when comment stripping is enabled.
Test Coverage
test/syntax.test.ts
Added four test cases to staticTestsWithComments validating multiline and single-line block-comment stripping behavior: bare /* export * */, multiline variants with newlines, and multiline with module.exports, ensuring proper ESM/CJS classification when comments are stripped.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Slashes and stars, now dancing right,
Across the lines, both dark and bright,
Multiline comments meet their fate,
Stripped away to work first-rate! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: handle multiline comments in stripComments' accurately describes the primary change: updating the regex to properly handle multiline block comments.
Linked Issues check ✅ Passed The PR directly addresses issue #279 by replacing .+? with [\\s\\S]*? in the block-comment regex to match newlines, fixing the multiline comment stripping bug described in the issue.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the multiline comment stripping issue: regex update in src/syntax.ts and targeted test cases in test/syntax.test.ts with no unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multiline comments are not stripped

1 participant