Add --all mode to serve-instrument package#1363
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new workspace package, @opendatacapture/serve-instrument-all, providing a dev server that discovers and serves all instruments under forms/ and interactive/, with an index page and per-instrument rendering/bundling.
Changes:
- Introduces the
serve-instrument-allCLI + HTTP server to enumerate instruments and serve an index plus per-instrument pages. - Adds a small React client/root app including an en/fr language switcher and instrument rendering via
ScalarInstrumentRenderer. - Adds build tooling (esbuild + Tailwind CSS extraction) and wires the new package into the pnpm workspace lockfile.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds the new workspace importer for packages/serve-instrument-all. |
| packages/serve-instrument-all/package.json | Defines the new package, bin entry, and dependencies for the CLI/dev server. |
| packages/serve-instrument-all/tsconfig.json | TypeScript config matching existing dev-server conventions (DOM libs, runtime path mapping). |
| packages/serve-instrument-all/scripts/build.js | Builds the Node CLI and browser client bundles (plus inlined Tailwind styles). |
| packages/serve-instrument-all/src/cli.ts | CLI argument parsing/validation and server startup (--port, --verbose). |
| packages/serve-instrument-all/src/server.tsx | HTTP server, instrument discovery, bundling/watch logic, and runtime asset serving. |
| packages/serve-instrument-all/src/root.tsx | React UI: index page + instrument page with language switcher. |
| packages/serve-instrument-all/src/client.tsx | Client hydration entrypoint for the server-rendered HTML. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const [, type, name] = instrumentMatch; | ||
| const decodedName = decodeURIComponent(name!); | ||
| const encodedBundle = await this.loaderMap.getEncodedBundle(type!, decodedName); |
| const clientBundle = await fs.promises | ||
| .readFile(path.resolve(import.meta.dirname, 'client.js'), 'utf-8') | ||
| .then((text) => `const __ROOT_PROPS__ = ${JSON.stringify(props)}; ${text}`); | ||
|
|
| fs.watch(target, { recursive: false }, () => { | ||
| log(chalk.yellow('↺') + chalk.dim(` [${this.label}] File changed, rebuilding...`)); | ||
| void this.updateEncodedBundle(); | ||
| }); |
| const [, type, name] = instrumentMatch; | ||
| const decodedName = decodeURIComponent(name!); | ||
| const encodedBundle = await this.loaderMap.getEncodedBundle(type!, decodedName); |
| const clientBundle = await fs.promises | ||
| .readFile(path.resolve(import.meta.dirname, 'client.js'), 'utf-8') | ||
| .then((text) => `const __ROOT_PROPS__ = ${JSON.stringify(props)}; ${text}`); | ||
|
|
| fs.watch(target, { recursive: false }, () => { | ||
| log(chalk.yellow('↺') + chalk.dim(` [${this.label}] File changed, rebuilding...`)); | ||
| void this.updateEncodedBundle(); | ||
| }); |
| const [, type, name] = instrumentMatch; | ||
| const decodedName = decodeURIComponent(name!); | ||
| const encodedBundle = await this.loaderMap.getEncodedBundle(type!, decodedName); |
| const clientBundle = await fs.promises | ||
| .readFile(path.resolve(import.meta.dirname, 'client.js'), 'utf-8') | ||
| .then((text) => `const __ROOT_PROPS__ = ${JSON.stringify(props)}; ${text}`); | ||
|
|
| fs.watch(target, { recursive: false }, () => { | ||
| log(chalk.yellow('↺') + chalk.dim(` [${this.label}] File changed, rebuilding...`)); | ||
| void this.updateEncodedBundle(); | ||
| }); |
joshunrau
left a comment
joshunrau
left a comment
There was a problem hiding this comment.
We should not be implementing a completely new package for this, with all of the code from serve-instrument copied near verbatim. You should use a subcommand instead so we can reuse the code and it can be maintainable in a single package.
Merges the multi-instrument serving capability directly into serve-instrument via a new --all CLI flag, instead of a separate serve-instrument-all package. With --all, the target directory must contain forms/ and/or interactive/ subdirectories. The server provides: - An index page linking to all discovered instruments - Per-instrument pages with hot-rebuild on file change - An en/fr language switcher - Verbose request logging Without --all, the existing single-instrument behavior is preserved.
gdevenyi
force-pushed
the
serve-instruments-all
branch
from
ab02f81 to
d2b92e5
Compare
gdevenyi
changed the title
Merged into serve-instrument package instead. |
Summary
Merges the multi-instrument serving capability directly into
serve-instrumentvia a new--allCLI flag.Usage
Single instrument (default, unchanged):
All instruments (new
--allmode):With
--all, the target directory must containforms/and/orinteractive/subdirectories. The server provides:fs.watch)-v)Changes
src/cli.ts: Added--all/-aflag; validates that target hasforms//interactive/subdirs in all modesrc/root.tsx: Merged UI —RootPropsis now a discriminated union (single|index|instrumentpages); addedLanguageSwitcherandIndexPagecomponentssrc/server.tsx: AddedInstrumentLoaderMapclass andAllModeHandleralongside existingSingleModeHandler;Server.createbranches onmodepackage.json: Bumped version to0.0.13