feat(gallery): promote scaffold gallery to root gallery/ app - #1371
Conversation
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
Review: root gallery app architecture, templates & CLI resolution
The promotion of the scaffold gallery to a top-level runnable app in /gallery cleanly establishes a single source of truth. Scaffolding, CLI prepack sync, and test suites are all aligned.
The new gallery/ workspace was never recorded, so every CI job died at `npm ci` with "can only install packages when your package.json and package-lock.json are in sync".
Promoting the gallery to a runnable root app gave it a root layout, a home page, a theme toggle, and cn.ts. copyGallery() copies app/, components/ and lib/ wholesale, so those four landed in every scaffolded app and the branch then skipped the scaffold's own versions to avoid the clobber. That silently downgraded 'webjs create': the generated layout lost displayName, cspNonce(), LayoutProps typing and the metadata.icons favicon, and cn.ts stopped being the verbatim @webjsdev/ui registry copy that 'webjs ui add' depends on. Filter the four out of both the copy and the prepack bundle instead, from one shared list, so monorepo-dev and installed-npm scaffolding emit byte-identical apps and the scaffold keeps writing its own shell. Gitignore the transient prepack bundle so it can never be committed back as a second source.
The new home page called cardClass({ interactive: true }) (it takes a class
string), badgeClass({ variant: 'secondary' }) (only default and outline
exist), and read f.category / f.description off NavItem, which carries
neither. Every card rendered two undefined holes, and webjs typecheck was
red on six errors.
Flatten FEATURE_GROUPS in the render function so a card can label itself
with the group it came from, and read the blurb the model actually has.
Also refresh cn.ts from the @webjsdev/ui registry: the file arrived as a
copy of the blog's older one, which is the drift promoting the gallery is
meant to end.
The gallery is promoted so framework changes get validated against it, but nothing ran it: its test:browser pointed at a bare wtr with no config and no runner dependency, and its node suite was in no job. Ship the scaffold's web-test-runner config, add the runner deps, and run typecheck plus 'webjs test' (node + browser) for gallery/ in the in-repo app job, next to website and blog. Migrate its database there first, because the auth test SKIPS rather than fails on a missing users table and would have gone green vacuously. Exclude **/browser/** from its tsconfig: those files run under the runner's mocha tdd globals, which are not in the app's type surface. Teach require-scaffold-with-src.sh the new gallery/ path, or the gate stops seeing a gallery demo as a teaching surface.
The image copied website and examples but not gallery/, so the artifact every service deploys could not serve it, and npm install inside the image had no manifest for the new workspace. Copy its manifest and source, compile its Tailwind alongside the others, and add the compose service (port 15005, its own SQLite volume) so local parity matches the intended gallery.webjs.dev service. Verified by building the image and booting the gallery from it: /__webjs/ready, the home page, a feature card, the auth card, and the todo example all 200, and the stylesheet is baked in.
gallery/ is now the canonical source of what every scaffolded app ships, so an agent editing it needs the rules that are specific to this directory: the generated templates copy is never authored, four app-shell files are not payload, and the gallery:clear + coverage-gate obligations that come with a new demo.
Review + readiness passPicked this up to get it mergeable. Every one of the 9 required checks was failing, and the review turned up a scaffold regression that would have shipped silently. Six commits on top. Blockers fixed1. 2. Promoting the gallery to a runnable app gave it The generator's own docstring stated the opposite invariant and was left in place: "The scaffold's own What a generated app lost: Fixed by filtering those four out of both the copy and the prepack bundle, from one shared list ( 3. The gallery home page was written against APIs that do not exist. Gaps closed against the issue's acceptance criteria
Doc surfaces synced: Verification
Left out, deliberately
One pre-existing gap this surfaced, not introduced here and not fixed here: a freshly scaffolded app's |
Five defects from the review of the readiness commits. The compose gallery service set only AUTH_SECRET, but the image sets NODE_ENV=production and the sessions card throws at module load without SESSION_SECRET, so every route under the sessions segment returned a 500 from the deploy artifact. The blog service two blocks above already sets both, for exactly this reason. FILE_URL_SECRET rides along because the file-storage card signs its download links and otherwise falls back to a dev value in production. .env.example documented neither, which is why the compose omission was easy to make, and gallery/AGENTS.md tells the reader that copying it is the whole first-run setup. hover:border-border-strong compiled to nothing: Tailwind v4 resolves border-<name> from a --color-<name> theme key, and the gallery's @theme block never mapped one, so the hover state did not exist on the home cards, the secondary button, or the todo rows. The generated scaffold already emits this mapping; only the gallery's own block was missing it. The home page hoisted a cardClass() call to module scope. That is a module side effect, so the page shipped to the browser with no client behaviour to justify it. Moving it into the render function fixes the half this PR introduced (nav.ts keeps the page shipping for a separate, pre-existing reason). npm run dev never started the gallery, so the port table added for it described a server that did not exist. It starts now, with a GALLERY_PORT override matching the other two apps.
Second review round, scoped to the six readiness commits (
|
The home page shipped to the browser in the gallery AND in every app
webjs create generates, for a page with no client behaviour at all. It
dragged card.ts, badge.ts, nav.ts and the ~400-line cn.ts along with it.
Two independent causes, both in modules/gallery/nav.ts.
FEATURES was a const initialised by a top-level FEATURE_GROUPS.flatMap()
call. A top-level call is a module side effect, so every importer was
pinned. It is a featureList() function now, called inside the render
function, and the generated home page calls it there too.
The second cause was the word "window" in the rate-limit blurb prose
("Retry-After past the window"). nav.ts is neither a route module nor a
component, so the elision analyser scans it with string literals intact
and the client-global pattern matched ordinary English inside a quoted
string. Reworded to "once the interval resets".
Verified: gallery app/page.ts goes shipped -> inert, and a freshly
generated app boots with app/page.ts absent from its boot script, 25
feature cards, and no undefined holes.
The analyser treating prose inside a string literal as a browser-global
reference is a framework-side false positive that outlives this reword.
Deferred finding fixed in-place (9a19005)The elision issue reported as out-of-scope above is fixed here rather than tracked separately. Investigating it showed my earlier diagnosis was only half right: there were two independent causes, both in 1. The top-level call. 2. The word "window" in prose. This is the one worth knowing about. The rate-limit blurb read Fixing only cause 1 left the verdict unchanged, which is why the first attempt looked like a no-op. Effect
This flows to the scaffold, since Repo suites after the change: Two things found while verifying, NOT changed hereThe analyser false positive outlives this reword. Any prose containing
CI note
|
The apps job became "In-repo app tests (website + blog + gallery)" in #1371, but scripts/protect-main.sh, and the protection it had applied, still required the old name, so that required check could never report and every PR since has shown as blocked on a check that no longer exists. Name the real job so a green run satisfies the gate.
…1472) * feat(cli): declare the webjs.ci step list and its reader Local CI (#1471) needs a step list an app declares once and every tool can read without importing app code, so it lives in the package.json `webjs` block next to the #550 dev/start orchestration. This lands the key in the three-surface lockstep (the JSON Schema with recursive definitions for a step, the WebjsConfig type, KNOWN_KEYS plus a nested guard for the recursive shape) and the pure CLI reader that normalizes the shorthand and reports every malformed entry with its JSON path. The reader validates the shapes itself because the boot validator only checks top-level key membership and never follows a $ref, and a step that is silently dropped is a check that never ran, the exact false green local CI exists to prevent. A group nested inside a parallel group takes one slot and runs sequentially, so a `parallel` on it is reported rather than honoured, the same rule Rails' runner applies. The runner and the `ci` command follow in the next commits. * feat(cli): add the local CI step runner The runner behind the ci command (#1471), modeled on Rails 8.1's ActiveSupport::ContinuousIntegration: a heading and a timed result line per step, a failure list plus one total line, fail-fast, and parallel groups whose steps run on N slots with output captured and replayed whole so nothing interleaves, a nested group taking one slot. Two spawn shapes on purpose. A sequential step inherits stdio and is not detached, so it owns the terminal and Ctrl-C reaches it natively, the split webjs dev already makes for before-steps versus watchers. A captured step is detached (its own process group, reaped on interrupt), has stdin ignored so a TTY-reading tool cannot stop on SIGTTIN and hang the pool, and resolves on close rather than exit, with a bounded grace so a leaked grandchild holding the pipe cannot hang the run. FORCE_COLOR reaches captured children only when the parent's stdout is a TTY; Node has no PTY without a native dependency. Under GitHub Actions each step is folded into a log group and a failure is annotated, so one cloud job running the whole list still names the layer that broke, which is what the per-layer jobs used to buy. Pure of process.exit, console, and the clock, so the slot cap, the fail-fast cutoff, replay atomicity, exit-then-data ordering, and the grace path are all proven with a scripted fake child. * feat(cli): add the ci command The local CI command (#1471): run the step list package.json declares under webjs.ci, with -f/--fail-fast, --only <title>, --json, and an opt-in --signoff that posts a green commit status through gh signoff after a green run, the Rails 8.1 bin/ci posture. The predicate is the config, not an app/ directory, unlike webjs check, because a workspace root is a legitimate target and this monorepo declares its own list. Nothing declared is exit 1 rather than 0, since a run of zero steps would read as green; the refusal names the workspace members that do declare one. A malformed block refuses with every problem's JSON path and runs nothing. Under --json stdout carries exactly one document and the human report moves to stderr; the exit code is set through exitCode so a non-TTY stdout is never truncated. .env is loaded before the steps, like dev and start, so a local db migrate step sees DATABASE_URL while a CI runner's explicit env still wins. The prose hook's CLI subcommand list gains ci so its drift test keeps passing, and the end-to-end CLI test and the Node + Bun proof script cover the exit codes, the output shape, the env every child sees, fail-fast, --only, the refusals, the GitHub Actions log groups and step summary, and a real interrupt reaping a detached sleep. * feat(cli): scaffold a local CI list and a one-job workflow that runs it Every new app now declares its CI once, in package.json under webjs.ci (#1471): a Setup step, then a Checks group running two at a time (webjs check, webjs doctor, webjs typecheck, a dependency audit) with a sequential Tests sub-group (the server, browser, and e2e layers), plus a `ci` script. The Tests group stays sequential because the server and e2e layers share one SQLite file. Every step is a bare `webjs ...` command, the same bar the before-steps meet, and a Bun app audits with bun audit. The generated GitHub workflow collapses from four hand-restated jobs to one job that prepares the runner and runs `npm run ci`, so the cloud and local lists cannot drift, which is the Rails guide's rule for every provider. The cloud practices stay: a read-only token, a bounded job, a concurrency cancel, and, through the runner's Actions mode, a log group per step, an annotation per failure, and a step table in the job summary, so a failure still names its layer. A team wanting per-layer required checks runs `webjs ci --only` in a matrix, as the workflow comment says. `webjs create --skip-ci` omits the workflow and nothing else (rails new parity); the local list always ships. The pre-commit hook is unchanged per #174: `npm run ci` is the pre-push gate, and the scaffold's agent rule files, playbooks, and PR template now say so. * feat: run local CI in the monorepo and its in-repo apps The framework dogfoods its own local CI (#1471). The root package.json declares a webjs.ci list mirroring the GitHub jobs (setup, the per-app check and doctor plus the two source invariants three at a time, the root test suite, the in-repo app typechecks and suites, the browser suite, the blog e2e, the Bun matrix) behind `npm run ci`, and gallery, examples/blog, and website each declare a shorter list behind their own `ci` script. The root script and every root step invoke this checkout's CLI by path rather than a hoisted bin, because in a linked worktree node_modules/.bin resolves into the primary checkout. The app lists go through their npm scripts because the website's pretest hook copies the ui registry, which a bare test invocation would skip. A repo-health test parses all four blocks through the same reader the command uses and checks that every npm script a step names exists in the package it targets, so a stale step fails here rather than at run time with a message that never mentions the block. The GitHub workflow itself is unchanged: its jobs are the required merge checks. * docs: document local CI across every surface The ci command and the webjs.ci config key (#1471) on each surface the doc-sync map names: the AGENTS.md CLI reference, config-block bullet, and code-workflow items; the agent skill (a Local CI section in built-ins.md, a one-command section in testing.md, a Testing Defaults bullet in SKILL.md); the docs site (a webjs ci entry under CLI Options on the configuration page with the step shapes, the flags, the Actions surfaces, and the signoff merge gate, a webjs ci command section on the testing page, a deployment-checklist item, and the known-key count); the root and CLI READMEs; and framework-dev.md for the monorepo's own list. * fix(cli): act on the review of the ci runner and command Eight findings from the review on #1472, each with its counterfactual. A captured step whose grandchild kept the pipe open resolved as truncated but left the group alive and the pipe handles open, so the bin printed its summary and then sat until the grandchild died. The grace bound now reaps the group (the child is detached, so it is still addressable) and destroys both pipes; the Node and Bun proof runs a `sleep & echo; exit 0` step and asserts the run settles through the grace and pgrep finds nothing. The reader honoured `parallel` on a group nested inside a sequential group while the schema, the type, and the docs all said a nested group never declares it. One rule now: a nested group refuses `parallel` whatever its parent, and `steps` is required on the schema, the type, and the reader alike (a block with no steps is refused, since zero steps would read as green). A nested group inside a pool bypassed the progress bookkeeping, so on a TTY its replays did not clear the line and no line showed while it ran, which is the scaffold's default shape (Tests inside Checks). Every step in a slot now shares one path. A second Ctrl-C exits with 130 outright, the way Rails' bin/ci lets a repeated interrupt through, so a captured child that ignores SIGTERM cannot trap the user. The total line reads "interrupted" after an interrupt instead of "failed" (the old expression was dead). A bare `--only`, or one followed by a flag, is refused instead of running the whole list. Colour follows NO_COLOR on a TTY, for the runner's own lines and for the FORCE_COLOR handed to captured children. The configuration page's free-form key list names `ci` and counts ten. * chore: require the renamed in-repo app tests check on main The apps job became "In-repo app tests (website + blog + gallery)" in #1371, but scripts/protect-main.sh, and the protection it had applied, still required the old name, so that required check could never report and every PR since has shown as blocked on a check that no longer exists. Name the real job so a green run satisfies the gate. * feat(create-webjs): forward --skip-ci to the scaffold `npm create webjs` and `bun create webjs` run this wrapper, so the flag `webjs create` gained in #1471 has to reach scaffoldApp from here too, or `npm create webjs my-app -- --skip-ci` would silently scaffold the workflow, the same gap --db once had. The usage and README name it, and a source-level test pins the forwarding, because the wrapper resolves the CLI by bare specifier and a linked worktree would run the primary checkout's copy. * perf: run the monorepo's local CI gate three steps at a time The root webjs.ci list ran its big steps one after another, so a full run took 9m22s on a 24-core machine although no single step needed more than 4m30s. After Setup, everything now sits in one Gate group with three slots, longest first (the blog e2e, the Bun matrix, the browser suite, then the root test suite, the in-repo app suites, and the Conventions checks as slots free up), so the wall-clock is the longest step rather than the sum. Each slot's output is captured and replayed whole, so nothing interleaves. Three rather than six: six overloaded the box, and web-test-runner's Firefox could not launch a test page inside its 30s start timeout. With the e2e alone as the floor, three slots cost no wall-clock. The Conventions group loses its own three-way parallelism, which cost nothing measurable (eight sub-second steps). The repo-health guard still parses the block clean, and framework-dev.md describes the shape.
1 participant
Closes #1370
Promotes the scaffold gallery from
packages/cli/templates/gallery/to a first-class, top-level runnable WebJs application in/galleryat the root of the monorepo.Summary of changes
gallery/(@webjsdev/gallery) with full routes, components, modules, and tests.scripts/sync-scaffold-gallery.mjsfor prepack/postpack sync.packages/cli/lib/create.jsto source templates directly fromgallery/in dev mode.Test Plan
webjs checkinsidegallery/.gallery/.webjs createscaffolding works cleanly.