fix: use strip-json-comments instead of fragile regex for JSONC parsing - #174
Open
YoinkingFishy wants to merge 1 commit into
Open
fix: use strip-json-comments instead of fragile regex for JSONC parsing#174YoinkingFishy wants to merge 1 commit into
YoinkingFishy wants to merge 1 commit into
Conversation
The hand-rolled removeJsonComments() regex strips // content even when it appears inside JSON string values (e.g. shell commands, file paths with // in them), causing valid settings.json files to be silently skipped. Replace with the strip-json-comments library which properly handles string context when stripping comments from JSONC content. Fixes firecrawl/firecrawl#3976
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="package.json">
<violation number="1" location="package.json:77">
P0: The added dependency strip-json-comments@^5.0.0 is ESM-only ("type": "module"), but the project compiles TypeScript to CommonJS ("module": "commonjs" in tsconfig.json). At runtime, require('strip-json-comments') will throw ERR_REQUIRE_ESM on any code path that calls stripJsonComments() — such as parsing Claude Code settings.json. Use strip-json-comments@^3.1.1 instead, which is published as CommonJS (no "type": "module") and compatible with the project's current module system.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| "@inquirer/prompts": "^8.2.1", | ||
| "commander": "^14.0.2", | ||
| "firecrawl": "4.24.0", | ||
| "strip-json-comments": "^5.0.0", |
There was a problem hiding this comment.
P0: The added dependency strip-json-comments@^5.0.0 is ESM-only ("type": "module"), but the project compiles TypeScript to CommonJS ("module": "commonjs" in tsconfig.json). At runtime, require('strip-json-comments') will throw ERR_REQUIRE_ESM on any code path that calls stripJsonComments() — such as parsing Claude Code settings.json. Use strip-json-comments@^3.1.1 instead, which is published as CommonJS (no "type": "module") and compatible with the project's current module system.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.json, line 77:
<comment>The added dependency strip-json-comments@^5.0.0 is ESM-only ("type": "module"), but the project compiles TypeScript to CommonJS ("module": "commonjs" in tsconfig.json). At runtime, require('strip-json-comments') will throw ERR_REQUIRE_ESM on any code path that calls stripJsonComments() — such as parsing Claude Code settings.json. Use strip-json-comments@^3.1.1 instead, which is published as CommonJS (no "type": "module") and compatible with the project's current module system.</comment>
<file context>
@@ -74,6 +74,7 @@
"@inquirer/prompts": "^8.2.1",
"commander": "^14.0.2",
"firecrawl": "4.24.0",
+ "strip-json-comments": "^5.0.0",
"yaml": "^2.9.0",
"zod-to-json-schema": "3.24.6"
</file context>
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.
Fixes firecrawl/firecrawl#3976
Problem
The hand-rolled
removeJsonComments()regex/(^|[^:\\\\])\/\/.*$/gmstrips everything after//on any line where//isn't preceded by:or\. This incorrectly mutates JSON string values containing//(e.g. shell commands, file paths), producing invalid JSON that causes Claude Code settings to be silently skipped.Change
removeJsonComments()with thestrip-json-commentslibrary which properly handles string context when stripping commentspackage.jsonto addstrip-json-comments@^5.0.0as a dependencyVerification
The function is used in a
try/catcharoundJSON.parse()— the change preserves the original error-handling behavior but makes parsing succeed for JSONC files where//appears inside string values.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Replaced fragile regex-based JSONC stripping with
strip-json-commentsto correctly parse configs when//appears inside strings, preventing valid Claude Code settings from being skipped. Fixes firecrawl/firecrawl#3976.Bug Fixes
strip-json-commentsfor comment removal, preserving string contents soJSON.parsesucceeds on JSONC.Dependencies
strip-json-comments@^5.0.0.Written for commit 7399c68. Summary will update on new commits.