Skip to content

feat(dom): jQuery wrapper scaffold + dom/query module (Step 1)#4399

Draft
elnelson575 wants to merge 15 commits into
mainfrom
feat/jquery-removal
Draft

feat(dom): jQuery wrapper scaffold + dom/query module (Step 1)#4399
elnelson575 wants to merge 15 commits into
mainfrom
feat/jquery-removal

Conversation

@elnelson575

Copy link
Copy Markdown
Contributor

Summary

First step of a long-term effort to wrap Shiny's jQuery usage behind a capability-oriented adapter layer. Lays the foundation; no user-facing API changes.

  • New srcts/src/dom/ wrapper. Capability modules with native + jQuery adapters and per-call dispatch (window.jQuery is checked at each call; native is used when absent).
  • First module: dom/query (select, closest, matches).
  • JQueryRequiredError for code paths that genuinely need jQuery and find it absent (infrastructure for follow-on modules; not yet thrown).
  • Build-time fence. srcts/build/_jquery.ts gains a second rule: import $ from "jquery" and JQuery<> type references are only allowed in srcts/src/dom/*/jquery.ts adapter files. A // shiny:jquery-allowed -- <reason> magic comment exempts pending files; every build prints a sorted audit of all 44 current exemptions so drift stays visible.
  • One proof-of-concept migration. $(document).find("[data-display-if]") in shinyapp.ts switched to select(document, "[data-display-if]"). Behavior preserved (dispatch picks jQuery in production).
  • Test infra. happy-dom added as devDep; setupDom() and withJQuery() helpers enable dual-path adapter parity tests. New tests: 12 parity (6 × 2 adapters) + 1 explicit no-jQuery regression + smoke tests for helpers/errors.

Design and plan are committed to the branch under .claude/specs/ and .claude/plans/.

Out of scope for this PR (deferred to follow-on PRs): dom/events, dom/html, dom/ajax, dom/widgets; the dual InputBinding/OutputBinding API; the package.json peer-dependency switch; migration of the 43 remaining jquery-importing files (each carries the escape-hatch comment for now).

Test plan

  • npm run checks passes locally (lint, build_types, test_types, coverage, circular).
  • npm run bundle_shiny succeeds and prints [_jquery audit] 44 file(s) carry shiny:jquery-allowed exemptions: at the end.
  • npx tsx --test srcts/src/dom/query/__tests__/query.test.ts reports 12 passes (6 logical × 2 adapters).
  • npx tsx --test srcts/src/dom/query/__tests__/query.no-jquery.test.ts passes with window.jQuery explicitly asserted undefined.
  • Smoke a Shiny app that uses conditionalPanel() (which relies on the data-display-if selection we migrated) to confirm no regression in the conditional-show behavior.
  • Inspect a deliberately-broken file by removing the shiny:jquery-allowed comment from any source file with import $ from "jquery" and confirm npm run bundle_shiny fails with the actionable Rule 2 error.

Notes for reviewers

  • Known limitation: withJQuery() does not await async callbacks; the first follow-on plan (dom/events) will need to extend it before tests that use await will work correctly. Surfaced in the final code review on this branch; not blocking this PR because no current code path is async.
  • dom/widgets future seam: JQueryRequiredError is the contract that the eventual dom/widgets module (jQuery-plugin widgets — datepicker, ion-rangeslider, selectize, DataTables) will throw when jQuery is absent. Defined here so adapter authors can rely on it.
  • The single migration in shinyapp.ts is intentionally narrow — the goal is to prove the dispatch works end-to-end, not to drain the queue.

Adds the design spec for wrapping Shiny's jQuery dependency behind a
small set of capability-oriented modules (dom/query, dom/events,
dom/html, dom/ajax, dom/widgets) with native + jQuery adapters and
per-call dispatch. Includes the optional-peer-dependency model, the
dual InputBinding/OutputBinding contract, build-time enforcement,
testing strategy, migration order, and an outline of future widget
replacements.
Rule 2: importing jquery or referencing JQuery<> types outside an adapter
file fails the build. A '// shiny:jquery-allowed -- <reason>' magic comment
exempts a file; exemptions are printed in an audit summary at the end of
every build, so drift is visible. All currently-jquery-using source files
are exempted with 'awaiting dom/* migration'; each follow-on plan removes
its module's exemptions as it migrates.
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