Skip to content

fix(cli): read an array preload back whatever quotes it is written in - #40

Merged
mhenrixon merged 1 commit into
mainfrom
fix/pin-option-parsing
Sep 19, 2026
Merged

mhenrixon merged 1 commit into
mainfrom
fix/pin-option-parsing

Conversation

@mhenrixon

@mhenrixon mhenrixon commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Packager#extract_existing_pin_options read an array preload: back through JSON.parse. Two shapes broke on it:

pin "x", preload: ["admin"]        => {preload: ["admin"]}
pin "x", preload: ["admin", "app"] => {preload: ["admin", "app"]}
pin "x", preload: []               => {}                       # dropped silently
pin 'x', preload: ['admin']        => JSON::ParserError: unexpected character

Single-quoted pins are a supported shape in this gem — test/packager_single_quotes_test.rb and the single_quote_* fixtures are there for it — so an app with pin 'x', preload: ['admin'] crashed on every command that reads a pin, not only the one rewriting it. And preload: [] never matched PRELOAD_OPTION_REGEXP (\[[^\]]+\] needs a character), so the next pin/update/pristine wrote the line back without it and restored the preload: true default on a package the app had asked to preload for no entry point.

Three additive changes in lib/importmap/packager.rb:

  • PRELOAD_OPTION_REGEXP[^\]]+[^\]]*, so [] matches.
  • preload_from_string — the entry points are scanned out of the literal (value.scan(/["']([^"']*)["']/)) instead of parsed. config/importmap.rb is Ruby, not JSON.
  • preloadreturn "" if preloads.nil? before Array(), which flattens nil and [] to the same thing; [] now writes preload: [].

Docs page updated: docs/app/views/docs/pages/pinning.rb ("Pin options survive a rewrite" now lists the array and empty-array forms and says quoting doesn't matter).
CHANGELOG entry under ## 1.2.0### Fixed.

Closes #28

Test plan

  • bundle exec rake test — 437 runs, 0 failures, with bun installed
  • --minify tests ran rather than skipped (test/minifier_test.rb 5 runs / 0 skips, commands_test.rb -n /minify/ 2 runs / 0 skips)
  • cd docs && bundle exec rake lint && bundle exec rspec — 53 files no offenses, 54 examples passed
  • New tests seen RED first on the pre-fix tree: JSON::ParserError for the single-quoted cases, {} and pin "react" for the empty-array ones
  • Manual: put pin 'md5', preload: ['admin'] and pin "crypt", preload: [] in an app's config/importmap.rb, run bin/importmap update md5, and confirm both lines come back with their preloads intact

Deviations & judgment calls

  • Scan instead of normalising quotes before JSON.parse. The issue offered either. The scan needs no JSON at all and also accepts the Ruby-isms JSON rejects (a trailing comma, [ ]), which is the right bar for a file that is Ruby.
  • A single-element array still normalises to a stringpreload: ["admin"] is rewritten as preload: "admin". Not fixed on purpose: the CLI's --preload admin arrives at preload() as exactly the same value (["admin"]), Map compares both through Array() so the meaning is identical, and test/packager_test.rb:79 has asserted the string form since upstream. Telling the two apart would mean moving CLI-shape normalisation out of preload() — a restructure of an upstream-owned method for no semantic gain.
  • No live commands_test.rb case. No new CDN contract and no new printed sentence; .claude/rules/testing.md says each live case costs a round-trip on every matrix cell. The round-trip is proven at unit level instead (read → pin_for → same line).
  • extract_existing_pin_options with single quotes is regression coverage, not proof. It passes on main too — a boolean preload with single quotes always worked. It is there because that test class covered only packaged? and remove. The array cases are the ones that were red.
  • pin --vendor silently discards a custom-URL pin and vendors from the resolved CDN instead #29 is a separate PR. It was picked in the same batch but lands in commands.rb on a different code path and has its own product decision; the diffs don't overlap.

Summary by cubic

Fixes array preload: handling when CLI commands read pins back from config/importmap.rb. Previously single-quoted pins like pin 'md5', preload: ['admin'] crashed with JSON::ParserError, and preload: [] was silently dropped, so pin, update, and pristine restored the preload: true default.

  • Scans entry points out of the Ruby literal instead of parsing it as JSON, so quote style no longer matters.
  • Preserves preload: [] in rewritten pins instead of omitting it.
  • Single-element arrays still normalize to a string (preload: ["admin"] becomes preload: "admin"), matching existing CLI behavior.
  • Adds regression tests, a fixture, a docs update, and a CHANGELOG entry.

Closes #28.

Written for commit 0018b3f. Summary will update on new commits.

Review in cubic

config/importmap.rb is Ruby, and this gem treats single-quoted pins as a
supported shape everywhere else — packager_single_quotes_test.rb and the
single_quote_* fixtures exist for exactly that. The preload option was the
one place that disagreed: it was read back through JSON.parse, so
`pin 'md5', preload: ['admin']` raised JSON::ParserError on every command
that reads a pin, not just the one rewriting it.

`preload: []` had a second problem in the same option. PRELOAD_OPTION_REGEXP
required at least one character between the brackets, so an empty array
matched nothing, extract_existing_pin_options returned no preload at all, and
the next pin/update/pristine wrote the line back without it — restoring the
`preload: true` default on a package the app had deliberately asked to
preload for no entry point. Array() flattens nil and [] alike, so the emitting
side had to learn the difference too.

The entry points are scanned out of the literal now instead of parsed, which
costs nothing and accepts every shape Ruby does. A single-element array still
normalises to a string (`preload: ["admin"]` -> `preload: "admin"`): the CLI's
`--preload admin` reaches the same method as the same value, Map compares both
through Array(), and packager_test.rb:79 has asserted it since upstream.

## Test coverage
- test/packager_test.rb: single-quoted, multi-entry and empty array preloads
  read back correctly; pin_for writes `preload: []` and still writes nothing
  for nil; a single-quoted array survives read-then-write as valid Ruby
- test/packager_single_quotes_test.rb: option extraction against a
  single-quoted map, which had no coverage at all
- test/fixtures/files/single_quote_array_preload_import_map.rb: the three
  array shapes as one fixture

Closes #28
@mhenrixon
mhenrixon merged commit 16ad4d7 into main Sep 19, 2026
47 checks passed
mhenrixon added a commit that referenced this pull request Sep 19, 2026
CHANGELOG.md was the only conflict: #40 and this branch each appended a
bullet to 1.2.0's "### Fixed" at the same spot. Union, main's bullet first.
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.

Single-quoted array preload crashes extract_existing_pin_options; empty array is dropped

1 participant