Emit valid JavaScript for every YAML document, and match ids on the path - #42
Open
toeknee-figma wants to merge 2 commits into
Open
Emit valid JavaScript for every YAML document, and match ids on the path#42toeknee-figma wants to merge 2 commits into
toeknee-figma wants to merge 2 commits into
Conversation
The serializer emitted code that would not run. A key like '07' became the
octal literal `{ 07: "foo" }`, which is a SyntaxError under the strict mode
every ES module uses, and a `__proto__` key reassigned the object's
prototype instead of defining a property — quoting the key does not change
that, only a computed key does (#30). Serialization now goes through
JSON.parse on a string literal, which engines parse faster than the
equivalent object literal and which cannot express either bug.
Timestamps, non-finite numbers and binary data do not survive JSON, so
those fall back to a literal that spells them out; a self-referencing
anchor now reports an error rather than silently emitting the
`{$circularReference:1}` placeholder tosource produced in place of the
data. That removes the last use of tosource, which was pinned to an alpha.
The `raw` option goes with it. It emitted `const data = [object Object];`
for any mapping and a bare identifier for any string, so it only ever
produced valid output for numbers and booleans, and the output is now
always both correct and compact.
Separately, the file extension was tested against the whole module id, so
an id carrying a query missed it and the raw YAML was passed on to be
evaluated as JavaScript. The extension and the include/exclude patterns
are now both matched against the path, and `?raw`, `?url` and the worker
queries are excluded deliberately rather than as a side effect of not
matching queried ids at all.
Scope of that second change, since an earlier version of this message
overstated it: Vite strips `?t=` and `?import` in its transform middleware
before the plugin pipeline runs, so those never reach the hook. Verified
against a real dev server on Vite 5 and 7 — the id arrives as a clean
path, and the previous code transformed it correctly. `?used` is the id
Vite can produce that still carries YAML and did hit the bug. This is
hardening; it is not the cause of #29, which is the plugin not being
registered in the Vite config that built the code.
Adds the test suite the repository never had, and runs it in CI along
with a typecheck. The strict-mode trap is the reason it needs one: the
old output parsed fine in a sloppy-mode eval and only failed inside a
module, so the tests evaluate the emitted code to catch it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
toeknee-figma
force-pushed
the
fix/transform-correctness
branch
from
August 1, 2026 23:52
9a77b11 to
abb24d5
Compare
`ViteYaml({ exclude: './translations/**' })` matches nothing. @rollup/pluginutils
tests patterns against absolute ids and resolves a non-absolute pattern against
process.cwd(), which is not reliably the project root — not in a monorepo, and
not when Vite is started with --config from another directory.
Measured against the same absolute id:
'./translations/**' excluded: false
'translations/**' excluded: false
'**/translations/**' excluded: true
resolve(root, './translations/**') excluded: true
So the fix offered to people hitting the plugin-conflict error did not work.
Recommend the anchored form, explain why, and cover it with a test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Second of three stacked PRs. Based on #41 — review that one first; the diff here is against it.
The serializer emitted code that would not run
'07': foo{ 07:"foo" }__proto__: {...}{ __proto__:{...} }foo: bar+raw: trueconst data = [object Object];hello+raw: trueconst data = hello;{$circularReference:1}Output now goes through
JSON.parseon a string literal, which engines parse faster than the equivalent object literal and which cannot express either of the first two bugs. Quoting__proto__does not help — only a computed key defines a property — so the literal fallback special-cases it.Values JSON cannot hold fall back to a literal that spells them out, so nothing regresses:
!!timestampstays aDate,.nan/.infstay non-finite numbers, and!!binarynow yields aUint8Arrayinstead of an object with numeric keys. A circular anchor raises a clear error rather than emitting a placeholder.This removes the last use of
tosource, which was pinned to an alpha.Removing
rawIt produced valid output only for numbers and booleans, and output is now always both correct and compact, so the option has nothing left to select. Discussed in #30. This is the breaking change behind the 2.0.0 bump in the next PR.
Matching ids on the path
The extension was tested against the whole module id, so an id carrying a query missed it and the raw YAML was passed on to be evaluated as JavaScript. The extension and the
include/excludepatterns are now both matched against the path, and?raw,?urland the worker queries are excluded deliberately rather than as a side effect of not matching queried ids at all.Scope, stated precisely this time. Vite strips
?t=and?importin its transform middleware before the plugin pipeline runs, so those never reach the hook. Against a real dev server on Vite 5.4.21 and 7.3.6, requesting/src/settings.yaml?t=1714951438630&importhandstransformthe clean path/src/settings.yaml, and the previous code transformed it correctly:?usedis the one id Vite can produce that still carries YAML, and it did hit the bug. So this change is hardening with a narrow blast radius, not a user-visible bug fix.What #29 actually is
Raw YAML reaching the browser as JavaScript because nothing transformed the file. Evaluating the reporter's exact document as a module reproduces both of their error strings:
That happens when the plugin is not in the Vite config that built the code — plausible for Quasar, which wraps Vite and has its own place to register plugins. The README troubleshooting entry has been rewritten to say this instead of blaming the query.
#16 is a separate matter and is addressed in #43.
Tests
The repository had none. This adds 34, plus typecheck and test steps in CI and a Node 22 matrix entry.
The strict-mode trap is why they are needed: the old output parses fine in a sloppy-mode
evaland only fails inside a module. The tests therefore evaluate the emitted code under an explicit strict directive, so output that is only broken in a module fails here rather than in someone's browser.🤖 Generated with Claude Code