From 2e1b94741cad42be82906365d5f2c9c5481e434b Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 10 Jun 2026 12:52:12 -0400 Subject: [PATCH 1/6] docs: design spec for flattening js/src (#54) --- .../specs/2026-06-10-flatten-js-src-design.md | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-10-flatten-js-src-design.md diff --git a/docs/superpowers/specs/2026-06-10-flatten-js-src-design.md b/docs/superpowers/specs/2026-06-10-flatten-js-src-design.md new file mode 100644 index 00000000..0d5b399e --- /dev/null +++ b/docs/superpowers/specs/2026-06-10-flatten-js-src-design.md @@ -0,0 +1,93 @@ +# Flatten and reorganize `js/src/` TS files + +**Issue:** [#54](https://github.com/posit-dev/shinyreact/issues/54) +**Date:** 2026-06-10 +**Type:** Refactor only — no public API change. The IIFE bundle output and `window.shinyreact` surface stay identical. + +## Goal + +Every top-level file under `js/src/` states its responsibility in one sentence, and `index.ts` reads as a boot sequence rather than a kitchen sink. + +## Background — what the issue already got, and what's left + +Issue #54's "Current layout" snapshot is stale; the directory evolved since it was filed. Status of its five proposals: + +| # | Proposal | Status | +|---|----------|--------| +| 1 | Extract `ShinyreactOutputBinding` + roots WeakMap | **Partial** — `roots.ts` already extracted; the binding class still lives in `index.ts` | +| 2 | Rename `spec.ts` → `types.ts` | **Not done** — `spec.ts` is still pure TS types | +| 3 | Colocate/consolidate the test | **Done** — a top-level `js/src/__tests__/` holds all three test files | +| 4 | Group global wiring into `global.ts` | **Not done** — `declare global` block + `window.shinyreact =` assignment still in `index.ts` | +| 5 | Vendored `shiny-react/` | Out of scope (un-vendoring is downstream of [#28](https://github.com/posit-dev/shinyreact/issues/28); reorganizing it in place is explicitly excluded) | + +This spec executes the remaining actionable work: **proposals 1, 2, and 4.** + +## Design + +### Target file layout + +``` +js/src/ + index.ts entry point — boot sequence only (~8 lines) + global.ts declares Window.shinyreact + installGlobal() assigns window.shinyreact (the public global API) + output-binding.ts ShinyreactOutputBinding class + registerShinyreactOutputBinding() + types.ts (was spec.ts) wire-tree TS types: Element, Spec, ComponentRegistry, … + registry.ts component registry (unchanged) + roots.ts React-root-per-element WeakMap (unchanged) + renderer.tsx wire-tree → React walker (unchanged) + inline-spec.tsx static-mount seeding + observer (unchanged) + shiny-output.tsx component (unchanged) + shiny.d.ts ambient Shiny global decls (doc comment updated to point at output-binding.ts) + shinyreact.css (unchanged) + __tests__/ (unchanged — proposal 3 already satisfied) + shiny-react/ (untouched — out of scope, #28) +``` + +### What moves where + +**1. `output-binding.ts` (new)** — the `ShinyreactOutputBinding` class, currently `index.ts:84–112`. Exports a `registerShinyreactOutputBinding()` function that performs the `Shiny.outputBindings.register(new ShinyreactOutputBinding(), "shinyreact.output")` call. The class itself need not be exported unless a test wants it. Imports `Spec` from `./types`, `ShinyreactRenderer` from `./renderer`, and `getOrCreateRoot` / `hasRoot` / `unmountRoot` from `./roots`. + +**2. `global.ts` (new)** — owns the `declare global { interface Window { shinyreact: {...} } }` block and the "why we bundle React into one IIFE" doc comment that explains it. Exports `installGlobal()`, which performs the `window.shinyreact = Object.assign(window.shinyreact || {}, { … })` assignment (preserving any pre-bundle assignment such as `window.shinyreact._restore`). Imports the re-exported hooks from `./shiny-react`, plus local `registerComponents`, `ShinyOutput`, `seedInlineSpecs`, `React`, `ReactDOM`. + +**3. `types.ts` (renamed from `spec.ts`)** — `git mv spec.ts types.ts`. Exported type *names* are unchanged (`Element`, `Spec`, `ComponentElement`, `TagElement`, `TextElement`, `HtmlElement`, `RegisteredComponentProps`, `ComponentRegistry`). Update the import path in the five importers: +- `renderer.tsx` +- `inline-spec.tsx` +- `registry.ts` +- `index.ts` (the `Spec` import disappears entirely once the binding moves out; verify after extraction) +- `__tests__/renderer.test.tsx` (`../spec` → `../types`) + +**4. `shiny.d.ts`** — update the doc comment ("Used only by the output binding in index.ts") to reference `output-binding.ts`. No declaration changes. + +### Resulting `index.ts` + +```ts +import "./shinyreact.css"; // side-effect import is how Vite bundles CSS — no alternative + +import { installGlobal } from "./global"; +import { registerShinyreactOutputBinding } from "./output-binding"; +import { installInlineSpecSeeding } from "./inline-spec"; + +installGlobal(); +registerShinyreactOutputBinding(); +installInlineSpecSeeding(); +``` + +### Design decisions + +- **No import-time side effects for our own modules.** Both `installGlobal()` and `registerShinyreactOutputBinding()` are explicit exported functions called from `index.ts`, so the boot order is visible at the entry point rather than hidden in import statements. (The `shinyreact.css` import is the one unavoidable side-effect import — it is how Vite includes the stylesheet in the bundle; there is no function-call equivalent.) +- **`installGlobal()` runs before `registerShinyreactOutputBinding()`** to preserve today's ordering (global assignment first, then binding registration, then inline-spec seeding). The seeding-last order matters for the `#123` registry-completeness reasoning already documented in `inline-spec.tsx`. + +## Out of scope + +- Any change to the IIFE entry point's external behavior or `window.shinyreact` surface. +- Changes to `vite.config.ts` / how the bundle is built. +- Anything inside `js/src/shiny-react/` (vendored upstream, #28). +- Renaming exported types (`Spec`, `Element`, `ComponentRegistry`, …) — only the file holding them is renamed. + +## Verification + +- `make js-build` produces a byte-similar `js/dist/shinyreact.js` (modulo bundler-internal source ordering); IIFE behavior and `window.shinyreact` surface identical. +- `make js-lint` (tsc `--noEmit`) passes. +- `cd js && npx vitest run` passes with no test logic changes beyond the one `../spec` → `../types` import path. +- Manual read: `index.ts` is a short boot sequence; each top-level `js/src/` file's responsibility is statable in one sentence (see layout table). +- After building, run `make update-dist` so `pkg-py/src/shinyreact/www/` and `pkg-r/inst/lib/shiny/` pick up the rebuilt bundle (even though it should be byte-similar). From 2996d347b082fc825d0e0fe5ee60969548218ab2 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 10 Jun 2026 12:55:12 -0400 Subject: [PATCH 2/6] docs: implementation plan for flattening js/src (#54) --- .../plans/2026-06-10-flatten-js-src.md | 387 ++++++++++++++++++ 1 file changed, 387 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-10-flatten-js-src.md diff --git a/docs/superpowers/plans/2026-06-10-flatten-js-src.md b/docs/superpowers/plans/2026-06-10-flatten-js-src.md new file mode 100644 index 00000000..3fb16dff --- /dev/null +++ b/docs/superpowers/plans/2026-06-10-flatten-js-src.md @@ -0,0 +1,387 @@ +# Flatten and reorganize `js/src/` TS files — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Shrink `js/src/index.ts` to a readable boot sequence by extracting the output binding and global wiring into their own files, and rename `spec.ts` → `types.ts`, with zero change to the IIFE bundle's external behavior. + +**Architecture:** Pure refactor of the IIFE entry module. Move three concerns out of `index.ts` — the `ShinyreactOutputBinding` class (→ `output-binding.ts`), the `window.shinyreact` global declaration + assignment (→ `global.ts`), and the wire-tree types (rename `spec.ts` → `types.ts`). Both extracted modules expose explicit `install`/`register` functions (no import-time side effects); `index.ts` calls them in order. Correctness is guaranteed by the existing test suite, `tsc --noEmit`, and a byte-similar build output — no new behavior, so no new tests. + +**Tech Stack:** TypeScript, React 19, Vite (IIFE build), Vitest, vendored `@posit/shiny-react`. + +**Spec:** `docs/superpowers/specs/2026-06-10-flatten-js-src-design.md` + +**Note on TDD for this plan:** This is a behavior-preserving refactor already covered by `js/src/__tests__/`. The discipline here is *keep the suite + lint green after every task and commit each step*. Each task ends by running `npx vitest run` and `npm run lint` (tsc) from `js/`, both of which must stay green. Do not add new tests — there is no new behavior. + +**Baseline (do this once before Task 1):** + +- [ ] Capture the current build output hash so we can confirm byte-similarity at the end. + +Run (from repo root): +```bash +cd js && npm run build && shasum dist/shinyreact.js && cd .. +``` +Expected: build succeeds; record the printed hash (call it `BASELINE_HASH`). The hash may legitimately differ slightly after the refactor due to bundler source ordering — the final check is "diff is trivial / module-order only", not "identical hash". + +- [ ] Confirm the suite and lint are green before starting. + +Run (from `js/`): +```bash +npm run lint && npx vitest run +``` +Expected: tsc reports no errors; all vitest tests PASS. + +--- + +## File Structure + +| File | Responsibility after refactor | +|------|-------------------------------| +| `js/src/index.ts` | Entry point — calls `installGlobal()`, `registerShinyreactOutputBinding()`, `installInlineSpecSeeding()` in order (~8 lines). | +| `js/src/global.ts` | **New.** `declare global { Window.shinyreact }` + `installGlobal()` performing the `window.shinyreact = Object.assign(...)` assignment. | +| `js/src/output-binding.ts` | **New.** `ShinyreactOutputBinding` class + `registerShinyreactOutputBinding()`. | +| `js/src/types.ts` | **Renamed** from `spec.ts`. Wire-tree TS types only; exported names unchanged. | +| `js/src/shiny.d.ts` | Ambient Shiny decls; doc-comment reference updated to `output-binding.ts`. | +| `js/src/registry.ts`, `roots.ts`, `renderer.tsx`, `inline-spec.tsx`, `shiny-output.tsx` | Unchanged except import path `./spec` → `./types` where present. | + +--- + +### Task 1: Rename `spec.ts` → `types.ts` + +Smallest mechanical change first; keeps later diffs clean. + +**Files:** +- Rename: `js/src/spec.ts` → `js/src/types.ts` +- Modify imports in: `js/src/renderer.tsx`, `js/src/inline-spec.tsx`, `js/src/registry.ts`, `js/src/index.ts`, `js/src/__tests__/renderer.test.tsx` + +- [ ] **Step 1: Rename the file via git** + +Run (from repo root): +```bash +git mv js/src/spec.ts js/src/types.ts +``` +Expected: no output; `git status` shows `renamed: js/src/spec.ts -> js/src/types.ts`. No edits inside the file — exported type names (`Element`, `Spec`, `ComponentRegistry`, …) stay as-is. + +- [ ] **Step 2: Update the five import paths** + +In each file, change the import source `"./spec"` → `"./types"` (and `"../spec"` → `"../types"` in the test). The exact lines: + +`js/src/renderer.tsx:2`: +```ts +import type { ComponentRegistry, Element, Spec } from "./types"; +``` +`js/src/inline-spec.tsx:2`: +```ts +import type { Spec } from "./types"; +``` +`js/src/registry.ts:1`: +```ts +import type { ComponentRegistry } from "./types"; +``` +`js/src/index.ts:3`: +```ts +import type { ComponentRegistry, Spec } from "./types"; +``` +`js/src/__tests__/renderer.test.tsx:4`: +```ts +import type { Spec } from "../types"; +``` + +- [ ] **Step 3: Verify lint and tests are green** + +Run (from `js/`): +```bash +npm run lint && npx vitest run +``` +Expected: tsc no errors (proves no dangling `./spec` import remains); all tests PASS. + +- [ ] **Step 4: Commit** + +```bash +git add js/src/types.ts js/src/renderer.tsx js/src/inline-spec.tsx js/src/registry.ts js/src/index.ts js/src/__tests__/renderer.test.tsx +git commit -m "refactor(js): rename spec.ts to types.ts (#54)" +``` + +--- + +### Task 2: Extract `output-binding.ts` + +Move the `ShinyreactOutputBinding` class out of `index.ts` into its own file with an explicit register function. + +**Files:** +- Create: `js/src/output-binding.ts` +- Modify: `js/src/index.ts` (remove the class + the `Shiny.outputBindings.register(...)` line; add an import + call — done fully in Task 3, but this task leaves `index.ts` calling the new function) + +- [ ] **Step 1: Create `js/src/output-binding.ts`** + +```ts +import React from "react"; +import type { Spec } from "./types"; +import { ShinyreactRenderer } from "./renderer"; +import { getOrCreateRoot, hasRoot, unmountRoot } from "./roots"; + +// Shiny output binding for .shinyreact-output elements. +class ShinyreactOutputBinding extends Shiny.OutputBinding { + find(scope: Element): ArrayLike { + return $(scope).find(".shinyreact-output"); + } + + renderValue(el: Element, data: Spec | null): void { + if (!data) { + if (hasRoot(el)) unmountRoot(el); + return; + } + const root = getOrCreateRoot(el as HTMLElement); + root.render(React.createElement(ShinyreactRenderer, { spec: data })); + } + + renderError(el: Element, err: { message: string }): void { + const root = getOrCreateRoot(el as HTMLElement); + root.render( + React.createElement( + "div", + { style: { color: "red", padding: "8px" } }, + err.message, + ), + ); + } +} + +/** + * Register shinyreact's output binding with Shiny. Shiny is always loaded + * before this runs because HTMLDependency ordering places Shiny's scripts + * first. + */ +export function registerShinyreactOutputBinding(): void { + Shiny.outputBindings.register( + new ShinyreactOutputBinding(), + "shinyreact.output", + ); +} +``` + +- [ ] **Step 2: Remove the class + register call from `index.ts`, wire in the new function** + +In `js/src/index.ts`: delete the `ShinyreactOutputBinding` class definition (the `class ShinyreactOutputBinding extends Shiny.OutputBinding { ... }` block) and the `Shiny.outputBindings.register(new ShinyreactOutputBinding(), "shinyreact.output");` line and its comment. Add an import near the other local imports: +```ts +import { registerShinyreactOutputBinding } from "./output-binding"; +``` +And replace the deleted register line with a call (keep it before the `installInlineSpecSeeding()` call): +```ts +registerShinyreactOutputBinding(); +``` +Also remove now-unused imports from `index.ts` that were only used by the class: `ShinyreactRenderer` (from `./renderer`), `getOrCreateRoot`/`hasRoot`/`unmountRoot` (from `./roots`), and the `Spec` type (from `./types`) if it is no longer referenced. (tsc in Step 3 will flag any that are still needed or any left dangling.) + +- [ ] **Step 3: Verify lint and tests are green** + +Run (from `js/`): +```bash +npm run lint && npx vitest run +``` +Expected: tsc no errors (no unused imports, no missing symbols); all tests PASS. + +- [ ] **Step 4: Commit** + +```bash +git add js/src/output-binding.ts js/src/index.ts +git commit -m "refactor(js): extract ShinyreactOutputBinding into output-binding.ts (#54)" +``` + +--- + +### Task 3: Extract `global.ts` and reduce `index.ts` to a boot sequence + +Move the `Window.shinyreact` declaration and assignment into `global.ts` behind an explicit `installGlobal()`. + +**Files:** +- Create: `js/src/global.ts` +- Modify: `js/src/index.ts` (becomes ~8 lines) + +- [ ] **Step 1: Create `js/src/global.ts`** + +```ts +import React from "react"; +import * as ReactDOM from "react-dom/client"; +import type { ComponentRegistry } from "./types"; +import { registerComponents } from "./registry"; +import { ShinyOutput } from "./shiny-output"; +import { seedInlineSpecs } from "./inline-spec"; + +// Re-export @posit/shiny-react hooks and components. +// +// We bundle @posit/shiny-react and React into this single IIFE so that: +// 1. All code shares a single React instance (React hooks break with multiple Reacts) +// 2. Downstream component authors get hooks via window.shinyreact.* +// 3. Downstream ESM builds can externalize React to window.shinyreact.React/ReactDOM +import { + useSetShinyInput, + useShinyBusy, + useShinyInput, + useShinyInputValue, + useShinyOutputStatus, + useShinyOutputValue, + useShinyMessageHandler, + useShinyInitialized, + ImageOutput, + MISSING, + ShinyModuleProvider, + ShinyReactComponentElement, +} from "./shiny-react"; + +// Extend window with shinyreact's public global API +declare global { + interface Window { + shinyreact: { + registerComponents: ( + catalog: unknown, + registry: ComponentRegistry, + ) => void; + useSetShinyInput: typeof useSetShinyInput; + useShinyBusy: typeof useShinyBusy; + useShinyInput: typeof useShinyInput; + useShinyInputValue: typeof useShinyInputValue; + useShinyOutputStatus: typeof useShinyOutputStatus; + useShinyOutputValue: typeof useShinyOutputValue; + useShinyMessageHandler: typeof useShinyMessageHandler; + useShinyInitialized: typeof useShinyInitialized; + ImageOutput: typeof ImageOutput; + MISSING: typeof MISSING; + ShinyModuleProvider: typeof ShinyModuleProvider; + ShinyReactComponentElement: typeof ShinyReactComponentElement; + ShinyOutput: typeof ShinyOutput; + seedInlineSpecs: typeof seedInlineSpecs; + React: typeof React; + ReactDOM: typeof ReactDOM; + }; + } +} + +/** + * Expose the public global API at `window.shinyreact`. Called once at bundle + * boot. Preserves any pre-bundle assignment (e.g. `window.shinyreact._restore` + * set by the head