Skip to content

alt: in-house JSONC patcher instead of comment-json - #1397

Open
jycouet wants to merge 1 commit into
feat/allow-json-commentsfrom
feat/allow-json-comments-own-jsonc
Open

jycouet wants to merge 1 commit into
feat/allow-json-commentsfrom
feat/allow-json-comments-own-jsonc

Conversation

@jycouet

@jycouet jycouet commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

@manuel3108 #1381 adds ~16% to the sv-utils bundle (+44 KB gzip, mostly esprima pulled in by comment-json), so I looked at alternatives with my friend Opus 5.5.

candidate gzip notes
comment-json (#1381) 36-50 KB reprints the whole file (expands inline arrays, drops trailing commas)
silver-fleece (main) 3 KB drops comments
@croct/json5-parser 10-13 KB ignores key reorder, minifies new files
jsonc-weaver ~150 KB wasm (jsonc-morph), ignores key reorder
aywson 7-10 KB breaks formatting
confbox 2 KB parses comments, doesn't keep them
jsonc-parser + diff 6-9 KB works, but broken ESM build / UMD main breaks Node + rolldown
own jsonc.ts (this PR) 2-3 KB minimal text edits, keeps comments/commas/inline arrays, handles reorder

Net: sv-utils ends up ~15 KB gzip smaller than main, all tests pass (+ fuzzed 30k random JSONC edits).

So, do we want to own these ~330 lines of JSONC? That is the question!

@pkg-svelte-dev

pkg-svelte-dev Bot commented Oct 3, 2026

Copy link
Copy Markdown

Install the latest version of sv from bb59023:

pnx https://pkg.svelte.dev/sv/c/bb5902362670b1f3dbf39456383b5e2086d05c78 create

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/cli/pr/1397

@changeset-bot

changeset-bot Bot commented Oct 3, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: bb59023

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@svelte-docs-bot

Copy link
Copy Markdown

@sacrosanctic

Copy link
Copy Markdown
Contributor

wow, can't believe we're going to publish our own jsonc parser. 😅

@manuel3108

Copy link
Copy Markdown
Member

I'm not a huge fan of this to be honest. Give that we have seriously considered #80 over the last couple of months, and that would add svelte's 5MB as a dependency, I don't see a reason we should bother with 44KB.
Also if we would do this, we should publish this separately for the benefit of the ecosystem.

Or we could beg Rich to move golden-fleece into the org and work on that one (silver-fleece, which we were using before, is based on Rich's work)

@jycouet

jycouet commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

With #80 (that I'll want one day too), I'm not sure what will be the bundle size.

What's triggering me is the size + 2 other deps we add on our shoulders and ship.
It's not a must to split and publish it, as the goal is to serve our needs and not a global thing.
I was happy to see that it's not in 1000+ lines territory and that's why I suggest.

Maybe we can look into adding in golden/silver/steel stuff 😅

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.

3 participants