posit-dev / posit-dev/shinyreact

Audit findings: parity gaps, latent bugs, and test holes found auditing FEATURES.md

Open
#223 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
14
Forks
3
Avg merge
9h 12m
Merged PRs (30d)
74

Description

FEATURES.md (the behavior tree) was audited leaf-by-leaf against all three packages, in both directions: is every claim true, and does the code do things no claim covers. The tree itself has been corrected. This issue tracks the code-level findings, which are real and unfixed.

Grouped by severity. Every item was verified against the source; the ones marked verified live were reproduced by running code.

Probably-wrong code (not just undocumented)

  1. [r] the #shinyreact-config tag renders in <body>, not <head>, under page_react() — verified live: renderTags(page_react()) puts it in $html, not $head. shinyreact_dep_page() returns a bare tagList (pkg-r/R/dep.R:28-30), whereas Python wraps it in head_content() (_bookmark.py:84). R's other path (page_react_html() → config_head_dep()) is head-correct, so R is inconsistent with itself as well as with Python. It works — the client finds the tag by id either way — but it is an unrecorded divergence.

  2. [r] page_react_html() does not preserve the document body verbatim — verified live: htmlTemplate() evaluates every {{ ... }} in the document as R code with parent = globalenv(), so a body containing {{ 6*7 }} renders 42. Any Vite/Handlebars document with {{ }} in the body is rewritten or errors. Python's marker handling is a plain replace and has no such behavior.

  3. [js] a forged window.shinyreact._restore silently disables restore and the protocol handshake — bookmark.ts:36-40 returns early on a truthy "-applied", before assertProtocolCompatible() runs, with no warning. Intended as idempotence; the side effect is a kill switch.

  4. [js] @posit/shinyreact (npm build) ships no CSS — npm.ts has no stylesheet import and package.json exposes no ./styles export or style field, so consumers get no @keyframes spin and ImageOutput's placeholder spinner does not animate. The IIFE bundle imports the CSS as a side effect; the ESM build does not.

  5. [js] output cleanup can remove the wrong DOM node — output-registry.ts:207 looks the div up with a document-global getElementById(outputId), so an id collision with user markup removes the user's node.

  6. [js] initializeMessageRegistry() is a silent no-op without window.Shiny, with no retry — so if Shiny loads after the first hook call, message handlers never register. The input registry deliberately tolerates a missing Shiny; this asymmetry looks unintended. message-registry.ts has zero tests.

  7. [r] utils is used but declared nowhere — getFromNamespace() (bookmark.R:40,49,57) and packageVersion() (dep.R:11) with no DESCRIPTION or NAMESPACE entry.

Parity gaps

  1. input-handler argument order differs: [py] (value, name, session) (Shiny's InputHandlerType), [r] (value, session, name). Both ignore the last two, so nothing catches it — but the signatures are documented as if shared.

  2. page_react_html() file encoding: [r] decodes UTF-8 explicitly (via htmlTemplate()); [py] uses Path.read_text() with no encoding=, i.e. the platform locale (_page.py:356,450).

  3. [r] restore precision is globally overridable: shiny:::toJSON's digits is getOption("shiny.json.digits", I(16)) — verified live: options(shiny.json.digits = 2) truncates 3.14159265358979 to 3.14 in the restore payload. Python has no equivalent global.

  4. [py] private-API use has no containment policy: reads session.output._outputs (_page.py:407) and imports shiny.bookmark._restore_state (_bookmark.py:6-9) inline. R confines its internals to named shiny___* wrappers with a shiny (>= 1.13.0) pin — though not totally: ctx$input$asList() (bookmark.R:81) calls a private R6 method directly.

Robustness / rough edges

  1. [r] a missing src_dir degrades to "." — verified live: no www/ yields <title>.</title> and a dependency named ., served at /lib/.-0/. No error, no test.
  2. [r] the marker check is fixed = TRUE, so {{headContent()}} is rejected even though htmlTemplate() would accept it.
  3. the marker check is not <head>-scoped in either language: a {{ headContent() }} in <body> passes and deps render there, though the error message asserts <head>.
  4. [py] ReactApp latches its mode at construction (_app.py:81), so creating/deleting www/index.html after startup never switches modes — even though the UI is per-request. Relatedly, the per-request UI re-runs page_react's existence checks and re-emits the missing-ui.js warning on every request.
  5. [py] static_assets edges, both untested: explicit static_assets=None still auto-mounts (the guard is is None), and a passed value replaces rather than merges, leaving the document's directory unserved.
  6. [js] useShinyOutputValue does not reset its held value when the id/namespace changes, so the old id's data stays visible.
  7. [js] MISSING does not cancel a pending debounced send, so a real value written just before it still lands on the server.
  8. [js] conflicting priority across mounts is silently last-writer-wins, while conflicting type throws.
  9. [js] initializeReactRegistry() is not idempotent (a second call discards all values); getReactRegistry() returns undefined unchecked when Shiny is present but init never ran, and throws otherwise; the output container is created with no document guard, so init throws in a DOM-less environment — the opposite of the lifecycle store's stated policy.
  10. [js] every manufactured output div ships the literal text This is the output div for <id> into the page.
  11. [js] ShinyReactComponentElement.disconnectedCallback calls unbindAll(this) without includeSelf, so the element itself is never unbound — the inverse of ShinyOutput's carefully reasoned choice.
  12. [js] the protocol handshake is skipped when protocolVersion is absent or falsy (config.ts:16-20, bookmark.ts:50), so a tagged page carrying no version silently skips version checking.

Test holes worth closing

  1. shiny-bound-output is asserted nowhere — it appears only as a comment in pkg-py/tests/playwright/test_shiny_output.py:24. One-line fix: expect(out).to_have_class(re.compile(r"\bshiny-bound-output\b")).
  2. [r] the bundle script's defer attribute is unpinned — Python pins its script attrs; R pins only src. This is precisely the #182 failure mode.
  3. the bundle-version fallback divergence is tested in neither language — [py] "0.1.0" vs [r] packageVersion("shinyreact").
  4. .tagify() in the renderer harvest is unpinned — test_react_page_harvests_session_output_deps still passes with the call deleted.
  5. page_react()'s "no body HTML" is asserted in neither language — and it is false in R (finding 1).
  6. __all__ / NAMESPACE export sets are pinned by no test in either language.
  7. [js] the 15-item window.shinyreact surface has no test — the object literal and the Window interface would drift together silently.
  8. Also unpinned: page_bare() emitting no config tag, set_react_page's explicit-missing-path error, renderer harvest in discovered mode, [r] send_message's invisible(NULL), [r] force = TRUE in .onLoad(), [js] debounceMs's numeric default, and every ImageOutput leaf except namespace precedence.

Not actionable, recorded for honesty

Twelve leaves are UNVERIFIABLE as written — rationale and policy claims with no code to check: the protocol's "bumps only when a boundary shape changes" (no test fails when it does not), <script defer> throwing on a module import, htmltools dep de-duplication, MISSING → SilentException (cross-language, unpinned), and "re-binding the parent is safe because Shiny skips bound elements" (mocked in unit tests). They are marked in the tree rather than dropped.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by splitting the findings by package and subsystem, then read the named entry points such as bookmark.ts, output-registry.ts, message-registry.ts, _page.py, and bookmark.R. Run the referenced tests, including pkg-py/tests/playwright/test_shiny_output.py, and add focused regression coverage for one selected finding. Done means the chosen behavior is corrected and its regression test passes.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, python, r, typescript
Domain
backend, frontend, testing
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.