Skip to content

Emit valid JavaScript for every YAML document, and match ids on the path - #42

Open
toeknee-figma wants to merge 2 commits into
chore/repo-metadata-and-depsfrom
fix/transform-correctness
Open

Emit valid JavaScript for every YAML document, and match ids on the path#42
toeknee-figma wants to merge 2 commits into
chore/repo-metadata-and-depsfrom
fix/transform-correctness

Conversation

@toeknee-figma

@toeknee-figma toeknee-figma commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Second of three stacked PRs. Based on #41 — review that one first; the diff here is against it.

Correction. An earlier version of this PR claimed the id-matching change fixed #29 and probably #16. That was wrong, and the commit message has been rewritten. Details in the last section — the serializer work below is unaffected and was verified independently.

The serializer emitted code that would not run

YAML before result
'07': foo { 07:"foo" } SyntaxError — octal literal, banned in the strict mode every ES module uses
__proto__: {...} { __proto__:{...} } reassigns the prototype instead of defining a key
foo: bar + raw: true const data = [object Object]; garbage
hello + raw: true const data = hello; ReferenceError
self-referencing anchor {$circularReference:1} placeholder silently replacing the data

Output now goes through JSON.parse on 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: !!timestamp stays a Date, .nan/.inf stay non-finite numbers, and !!binary now yields a Uint8Array instead 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 raw

It 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/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, stated precisely this time. Vite strips ?t= and ?import in 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&import hands transform the clean path /src/settings.yaml, and the previous code transformed it correctly:

/src/settings.yaml                        -> TRANSFORMED by old plugin
/src/settings.yaml?t=1714951438630&import -> TRANSFORMED by old plugin
/src/settings.yaml?import                 -> TRANSFORMED by old plugin

ids actually handed to transform():
    ./src/settings.yaml

?used is 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:

with --- (their second error)    -> SyntaxError: Invalid left-hand side expression in prefix operation
without --- (their first error)  -> parses OK; exports: []   ("does not provide an export named 'default'")

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 eval and 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

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
toeknee-figma force-pushed the fix/transform-correctness branch from 9a77b11 to abb24d5 Compare August 1, 2026 23:52
@toeknee-figma toeknee-figma changed the title Fix YAML files being skipped on reload and emitted as invalid code Emit valid JavaScript for every YAML document, and match ids on the path Aug 1, 2026
`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>
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.

1 participant