Skip to content

add_html_dependency: implement version, warn once per dependency (bd-add-html-dependency-version-5tnub5ds) - #531

Merged
cscheid merged 4 commits into
mainfrom
bugfix/bd-add-html-dependency-version-5tnub5ds
Aug 14, 2026
Merged

add_html_dependency: implement version, warn once per dependency (bd-add-html-dependency-version-5tnub5ds)#531
cscheid merged 4 commits into
mainfrom
bugfix/bd-add-html-dependency-version-5tnub5ds

Conversation

@cscheid

@cscheid cscheid commented Aug 14, 2026

Copy link
Copy Markdown
Member

Fixes bd-add-html-dependency-version-5tnub5ds. Plan:
claude-notes/plans/2026-08-14-add-html-dependency-version.md.

The two fixes

1. The warning fired once per call, not once per dependency. The
field-validation loop in add_html_dependency ran before the
dedup-by-name early-return. Extensions are written to call the function
unconditionally for every matching element — that's the documented idiom,
and the dedup is what makes it safe — so every call warned even though only
the first registers anything. The Posit Connect docs' mermaid-zoom
extension produced 33 identical warnings across 14 pages.

The loop is split rather than moved wholesale: unknown fields still
hard-error on every call (a typo is a bug regardless of whether that
particular call would have registered), while the unsupported-field warning
moves after the early-return.

2. version is implemented instead of warning. Assets nest at
libs/{name}/{version}/{file}, and the artifact key gains the version;
unversioned dependencies keep libs/{name}/{file} byte-identical.

Both halves of (2) matter, and the key is the one that isn't obvious. It is
what lets two renders producing different versions of one dependency keep
both sets of assets — the requirement freeze will depend on, and the only
reason the field is honored at all. Without it the directory would be
versioned while the artifacts still collapsed, which would look correct on
disk and silently defeat the purpose.

Quarto 1 puts these under quarto-contrib/{name}-{version}/; q2
deliberately does not mirror that (no notion of "external" dependencies, and
no _site layout longevity promise). Within a single document the dedup
stays keyed on name alone so the call-once-per-element idiom is still
silent, but registering one name at two different versions now warns — a
page cannot sensibly load two copies of one library.

Docs

quarto.doc.add_html_dependency had no user-facing coverage; this adds a
section to the Lua filters guide (docs/guides/authoring/lua-filters.qmd),
which already documents adjacent API.

Verification

  • 13 new tests, each written first and confirmed failing behaviorally before
    implementation.
  • End-to-end through the binary (per CLAUDE.md): q2 render on the
    committed repro gives zero warnings (was 2), asset at
    _site/site_libs/libs/versioned-dep/1.0.0/versioned-dep.js, and the
    emitted <script src> agrees with the on-disk path. Output inspected, not
    inferred from exit status.
  • Docs verified through the real renderer (q2 render docs/, 239/239).
  • Full cargo xtask verify green on the pre-rebase base (14/14, WASM leg
    included). After rebasing onto current main, cargo xtask verify --skip-hub-build is green (12079 Rust tests, hub-client tests included).

Why the WASM leg is skipped locally: current main (1eeaae5d,
live-share payload reduction) added a hard wasm-opt/binaryen prerequisite
that isn't installed on this machine, so npm run build:wasm fails before
reaching any of this branch's code. Unrelated to these changes — CI has the
tool. Same story for
config_endpoint::config_reports_embedded_asset_manifest_hashes, which
fails identically on unmodified origin/main until cargo xtask build-q2-preview-spa refreshes the embedded dist.

No snapshot files added, modified, or removed.

Follow-ups filed

  • bd-k2ox4tqq — cross-document diagnostic dedup. The Lua state is per
    (document, filter, pass), so this fix takes the Connect docs from 33
    warnings to 14, not to 1. Getting to 1 is new infrastructure, deliberately
    out of scope.
  • bd-mcy2316m — a script/stylesheet entry's name sub-field is accepted
    and silently ignored (Q1 uses it as the output filename). Found while
    writing the docs; the docs text deliberately doesn't promise renaming.

🤖 Generated with Claude Code

…ncy version + Q-11-1 per-call warning

Plan skeleton plus a committed repro. Key findings that reshape the
strand's own suggested fix:

- The Lua state is per (document, filter, pass), so the "move the field
  check after the dedup" reorder takes the Connect docs from 33 warnings
  to 14, not to 1. Point 2 alone does not resolve the reported symptom.
- Q1's `version` buys directory naming only, not multi-version
  coexistence: Q1's Lua side does no dedup and the TS side dedups by
  name before consulting version.
- Full Q1 path parity needs `quarto-contrib/` as well as the version
  suffix; q2 has no notion of external deps at all.

No snapshot files added, modified, or removed.
version will be implemented, not silently ignored: freeze needs
multi-version dependency coexistence so an old frozen page's assets
are not clobbered by a newer render. This amends the investigation's
Finding 2 — Q1's name-only dedup blocks coexistence within a render,
but the case version serves is across renders, so q2 should not copy
that dedup into the versioned world.

New disk layouts accepted (no _site longevity promise), so
quarto-contrib/ is not required. Cross-document diagnostic dedup filed
separately as bd-k2ox4tqq. Field loop to be split; other unsupported
fields keep warning.

Two questions remain open: whether version participates in the Lua-side
dedup key as well as the artifact key, and flat vs nested layout.

No snapshot files added, modified, or removed.
…ce freeze

Dedup keys: name-only for the intra-document Lua scan (prevents
double-injection on one page, first-wins plus a warning on a
same-name/different-version collision), (name, version) for the
artifact key (preserves cross-render coexistence, which is what freeze
needs). Layout: nested libs/{name}/{version}/{file}; unversioned deps
keep libs/{name}/{file}.

Also surveyed the freeze design record, since this strand honors
version solely on freeze's behalf. There is no freeze epic — only
bd-mx5x609r, a p4 "when freeze lands" reminder. Recorded where freeze
is anticipated (document-profile-contract.md, q2-preview-epic,
websites-phase-9) and flagged a divergence for the eventual design to
resolve: multi-engine-execution.md:349-353 records freeze as
trace-replay ("engine: replay as freeze"), while the 2026-08-14
discussion described reworking the execution-output automerge sidecar
into a portable .ipynb-based format. Not resolved here; the plan states
the single requirement this work depends on either way.

No snapshot files added, modified, or removed.
Two fixes for bd-add-html-dependency-version-5tnub5ds.

1. The field-validation loop ran before the dedup-by-name early-return,
   so every call warned even though only the first registers anything.
   Extensions are written to call add_html_dependency once per matching
   element (that is the documented idiom), so this turned one distinct
   diagnostic into a flood — 33 warnings across 14 pages in the Posit
   Connect docs.

   The loop is now split rather than moved wholesale: unknown fields
   still hard-error on every call (a typo is a bug regardless of
   whether that call would have registered), while the
   unsupported-field warning moves after the early-return and so fires
   once per distinct dependency.

2. `version` is implemented instead of warning. Assets nest under
   libs/{name}/{version}/{file} and the artifact key gains the version;
   unversioned dependencies keep libs/{name}/{file} unchanged.

   Both halves of (2) matter. The key is what lets two renders that
   produce different versions of one dependency keep both sets of
   assets — the requirement `freeze` will depend on, and the whole
   reason the field is honored. Quarto 1 puts these under
   quarto-contrib/{name}-{version}/; q2 deliberately does not mirror
   that (no notion of external deps, no _site layout promise).

   Within a single document the dedup stays keyed on name alone, so the
   call-once-per-element idiom is still silent, but registering one name
   at two different versions now warns — a page cannot sensibly load two
   copies of one library.

Also documents quarto.doc.add_html_dependency in the Lua filters guide,
which had no coverage of it at all.

Verified end-to-end through `q2 render` on the committed repro: zero
warnings (was 2), asset at site_libs/libs/versioned-dep/1.0.0/, and the
emitted <script src> agrees with the on-disk path. Full `cargo xtask
verify` green (14/14, WASM leg included).

Follow-ups filed: bd-k2ox4tqq (cross-document diagnostic dedup — the
residual one-warning-per-page is out of scope here), bd-mcy2316m (a
script/stylesheet entry's `name` sub-field is silently ignored, found
while writing the docs).

No snapshot files added, modified, or removed.
@posit-snyk-bot

posit-snyk-bot commented Aug 14, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@cscheid
cscheid merged commit d29f594 into main Aug 14, 2026
8 checks passed
@cscheid
cscheid deleted the bugfix/bd-add-html-dependency-version-5tnub5ds branch August 14, 2026 19:19
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.

2 participants