Conversation
Member
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
fengmk2
changed the title
fengmk2
force-pushed
the
merge-json-config-support-jsonc-format
branch
from
5dd3d8a to
1dfdbcc
Compare
fengmk2
force-pushed
the
merge-json-config-support-jsonc-format
branch
from
1dfdbcc to
157a975
Compare
fengmk2
changed the base branch from
graphite-base/378
to
01-04-test_e2e_skip_rollipop_lint_check
This was referenced Jan 4, 2026
This was referenced Jan 4, 2026
Closed
fengmk2
changed the base branch from
01-04-test_e2e_skip_rollipop_lint_check
to
graphite-base/378
fengmk2
force-pushed
the
graphite-base/378
branch
from
402e5df to
0971535
Compare
fengmk2
force-pushed
the
merge-json-config-support-jsonc-format
branch
from
157a975 to
587ea35
Compare
fengmk2
force-pushed
the
merge-json-config-support-jsonc-format
branch
from
587ea35 to
070a557
Compare
fengmk2
force-pushed
the
merge-json-config-support-jsonc-format
branch
from
070a557 to
4a5de30
Compare
Use json-strip-comments crate to strip comments before parsing, enabling support for .jsonc config files with // and /* */ comments. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Simplify implementation by using JSON/JSONC content directly as valid JS. No parsing or conversion needed - just read file and merge. - Remove json-strip-comments dependency (not needed) - Remove json_to_js_object_literal and helper functions - Comments in JSONC files are preserved in output 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
fengmk2
force-pushed
the
merge-json-config-support-jsonc-format
branch
from
4a5de30 to
65ceee6
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Vite configuration migration process by treating JSON/JSONC content as valid JavaScript instead of parsing and converting it. The approach leverages the fact that JSON is valid JavaScript syntax, and comments (from JSONC) are also valid in JavaScript contexts.
- Removes ~130 lines of JSON-to-JavaScript conversion code including helper functions for escaping, identifier validation, and reserved word checking
- Adds comprehensive test coverage for JSONC comment preservation (single-line, block, and inline comments)
- Removes the TODO comment about handling JSONC files, as the new approach naturally supports them
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/global/src/migration/migrator.ts | Removes TODO comment about JSONC handling since the refactored approach now handles it |
| packages/global/snap-tests/migration-monorepo-yarn4/snap.txt | Updates test snapshot to reflect double-quoted JSON style in merged config |
| packages/global/snap-tests/migration-monorepo-pnpm/snap.txt | Updates test snapshots for both root and package configs with double-quoted JSON style |
| packages/global/snap-tests/migration-merge-vite-config-ts/snap.txt | Updates test snapshot to show merged config with JSON-style double quotes |
| packages/global/snap-tests/migration-merge-vite-config-js/snap.txt | Updates test snapshot for JavaScript config with double-quoted merged content |
| packages/global/snap-tests/migration-from-tsdown-json-config/snap.txt | Updates snapshots to show tsdown config merges using double-quoted JSON style |
| packages/global/snap-tests/migration-auto-create-vite-config/snap.txt | Updates snapshot for auto-created config with double-quoted merged JSON content |
| crates/vite_migration/src/vite_config.rs | Simplifies JSON merging by using raw file content, removes conversion logic, adds JSONC tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Brooooooklyn
approved these changes
Jan 6, 2026
Member
Author
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.
Improves the Vite config migration process by directly using JSON/JSONC content in JavaScript context instead of parsing and converting it. Since JSON is valid JavaScript, this approach:
//and/* */style)The PR includes tests to verify that comments in JSONC files are properly preserved during migration.
Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com