diff --git a/package.json b/package.json index dc3034939..b2316be6c 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,8 @@ "files": [ "dist/bin.cjs", "dist/index.cjs", - "dist/index.d.cts" + "dist/index.d.cts", + "dist/ink-app.js" ], "license": "FSL-1.1-Apache-2.0", "packageManager": "bun@1.3.13", diff --git a/plugins/sentry-cli/skills/sentry-cli/references/init.md b/plugins/sentry-cli/skills/sentry-cli/references/init.md index 4fe4d9bc5..451036d95 100644 --- a/plugins/sentry-cli/skills/sentry-cli/references/init.md +++ b/plugins/sentry-cli/skills/sentry-cli/references/init.md @@ -20,7 +20,7 @@ Initialize Sentry in your project (experimental) - `-n, --dry-run - Show what would happen without making changes` - `--features ... - Features to enable: errors,tracing,logs,replay,metrics,profiling,sourcemaps,crons,ai-monitoring,user-feedback` - `-t, --team - Team slug to create the project under` -- `--tui - Use the Ink-based interactive UI (default on the Bun binary). Pass --no-tui to fall back to plain log output.` +- `--tui - Use the Ink-based interactive UI (default). Pass --no-tui to fall back to plain log output.` **Examples:** diff --git a/script/bundle.ts b/script/bundle.ts index 10e1cc94a..9477b6386 100644 --- a/script/bundle.ts +++ b/script/bundle.ts @@ -216,18 +216,14 @@ const result = await build({ "import.meta.url": "import_meta_url", }, // Externalize Node.js built-ins, plus Ink + React + companions. - // Ink uses top-level await (in `node_modules/ink/build/reconciler.js` - // and `yoga-layout/dist/src/index.js`) which esbuild can't emit in - // a CJS bundle, so the packages must stay external for the - // npm/Node distribution. The factory in `factory.ts` lazy-imports - // the Ink path via `with { type: "file" }` and falls back to - // `LoggingUI` on import failure, so a Node user without Ink - // installed simply gets the non-TUI flow without a crash. - // - // The Bun compile (`script/build.ts`) embeds `ink-app.tsx` as a - // file resource — at runtime Bun's loader resolves Ink + React - // fresh, sidestepping the same CJS-wrapping bug that'd hit if - // these were bundled into the binary's pre-compiled JS. + // These packages are NOT bundled into the main CJS output because + // they use top-level await (esbuild can't emit that in CJS). + // Instead, the Ink UI lives in a separate self-contained ESM + // sidecar (`dist/ink-app.js`) that the text-import-plugin + // pre-bundles with all deps inlined. The main bundle references + // the sidecar via a path string and loads it lazily via dynamic + // `import()` at runtime. The external list here prevents esbuild + // from trying to resolve these packages in the main bundle graph. external: [ "node:*", "ink", @@ -300,18 +296,11 @@ await Bun.write("./dist/index.d.cts", TYPE_DECLARATIONS); console.log(" -> dist/bin.cjs (CLI wrapper)"); console.log(" -> dist/index.d.cts (type declarations)"); -// Clean up the `ink-app.js` sidecar that the text-import-plugin -// drops into `dist/` when it pre-bundles the `with { type: "file" }` -// import in `src/lib/init/ui/ink-ui.ts`. The npm distribution -// doesn't run the InkUI factory at all (it's gated to the Bun -// binary because Ink uses top-level await that we can't bundle -// into CJS), so the sidecar is unused — removing it just keeps the -// local `dist/` directory tidy. -try { - await unlink("./dist/ink-app.js"); -} catch { - // Sidecar may not exist (e.g. plugin path not exercised) — fine. -} +// The `ink-app.js` sidecar (pre-bundled by text-import-plugin) ships +// with the npm package so `npx sentry@latest init` can load the +// interactive Ink UI on Node via dynamic import(). The sidecar is +// self-contained ESM with all deps inlined — no runtime dependencies +// needed. // Calculate bundle size (only the main bundle, not source maps) const bundleOutput = result.metafile?.outputs["dist/index.cjs"]; diff --git a/script/text-import-plugin.ts b/script/text-import-plugin.ts index c99bdc421..01ce63329 100644 --- a/script/text-import-plugin.ts +++ b/script/text-import-plugin.ts @@ -47,6 +47,7 @@ import { basename, dirname, extname, resolve as resolvePath } from "node:path"; import { build as esbuildBuild, type Plugin } from "esbuild"; const TEXT_IMPORT_NS = "text-import"; +const FILE_PATH_NS = "file-path-import"; const ANY_FILTER = /.*/; /** Extensions that need TypeScript/JSX transpilation before embedding. */ @@ -127,10 +128,24 @@ export const textImportPlugin: Plugin = { ); } + // For CJS bundles (npm distribution), emit a virtual module that + // exports the sidecar filename as a string. The consumer resolves + // the full path at runtime using import.meta.url. For ESM bundles + // (Bun binary build), mark external so Bun.compile embeds the file. + if (build.initialOptions.format === "cjs") { + return { + path: outFilename, + namespace: FILE_PATH_NS, + }; + } return { path: `./${outFilename}`, external: true }; } return null; }); + build.onLoad({ filter: ANY_FILTER, namespace: FILE_PATH_NS }, (args) => ({ + contents: `export default ${JSON.stringify(`./${args.path}`)};`, + loader: "js", + })); build.onLoad({ filter: ANY_FILTER, namespace: TEXT_IMPORT_NS }, (args) => { const content = readFileSync(args.path, "utf-8"); return { diff --git a/src/commands/init.ts b/src/commands/init.ts index 47e940893..2d62af077 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -82,13 +82,11 @@ type InitFlags = { readonly features?: string[]; readonly team?: string; /** - * Default `true` (Ink is the default UI on the Bun binary). Stricli - * auto-generates a negated `--no-tui` flag that flips this to - * `false` — that's the escape hatch users invoke when the Ink path - * misbehaves (e.g. on unusual terminal emulators). The positive - * `--tui` flag is also accepted for symmetry but is a no-op versus - * the default. On the npm/Node distribution this flag has no - * effect; the factory always picks `LoggingUI` there. + * Default `true` — Ink is the default UI on both the Bun binary + * and the npm/Node distribution. Stricli auto-generates a negated + * `--no-tui` flag that flips this to `false` — that's the escape + * hatch users invoke when the Ink path misbehaves (e.g. on unusual + * terminal emulators). */ readonly tui: boolean; }; @@ -340,7 +338,7 @@ export const initCommand = buildCommand< tui: { kind: "boolean", brief: - "Use the Ink-based interactive UI (default on the Bun binary). Pass --no-tui to fall back to plain log output.", + "Use the Ink-based interactive UI (default). Pass --no-tui to fall back to plain log output.", default: true, }, }, diff --git a/src/lib/init/ui/factory.ts b/src/lib/init/ui/factory.ts index 192c3bad4..f90ab5819 100644 --- a/src/lib/init/ui/factory.ts +++ b/src/lib/init/ui/factory.ts @@ -16,21 +16,17 @@ * context this means the wizard becomes effectively non-interactive * (any prompt aborts), so users hitting this path will need to set * every choice via flags or rely on auto-detection. - * 3. Running outside the Bun-compiled binary (i.e. on Node) — also - * `LoggingUI`. Ink uses top-level await in its reconciler and the - * `yoga-layout` dependency, which esbuild can't emit in our CJS - * bundle, so the npm distribution can't load Ink at runtime. The - * Bun binary embeds Ink + React + ink-app.tsx via - * `with { type: "file" }`, sidestepping the bundler entirely. The - * npm package's `--help` output and onboarding docs direct users - * to the Bun binary for the interactive `sentry init` experience. - * 4. Default (Bun binary, interactive, no opt-out) — `InkUI`. + * 3. Default (interactive, no opt-out) — `InkUI`. Works on both the + * Bun binary (Ink embedded via `with { type: "file" }`) and the + * npm/Node distribution (self-contained ESM sidecar loaded via + * dynamic `import()`). Falls back to `LoggingUI` if the import + * fails for any reason. * * Implementation history: * - PR 4: replaced `ClackUI` with `OpenTuiUI` as the default. * - This PR: replaced `OpenTuiUI` with `InkUI`. OpenTUI's Zig - * bindings added ~10.7 MB to the binary; Ink + React + companions - * add a fraction of that and use no native code. + * bindings added ~10.7 MB to the compiled binary; Ink + React + + * companions add a fraction of that and use no native code. */ import { LoggingUI } from "./logging-ui.js"; @@ -58,12 +54,8 @@ export type UIFactoryOptions = { /** * Detect whether the CLI is running inside the Bun-compiled binary - * (where the embedded `ink-app.tsx` resource is reachable) vs. the - * npm/Node distribution. The `Bun` global only exists in the Bun - * runtime. - * - * Exported for the test suite — production callers should go through - * `getUIAsync()`. + * vs. the npm/Node distribution. The `Bun` global only exists in + * the Bun runtime. */ export function isBunRuntime(): boolean { return ( @@ -85,8 +77,8 @@ export function isInteractiveTerminal(): boolean { /** * Returns `true` when the `LoggingUI` should be used — i.e. we're in - * a non-interactive context, the user opted out of the TUI, the env - * var override is set, or the runtime can't load Ink. + * a non-interactive context, the user opted out of the TUI, or the + * env var override is set. */ function shouldUseLogging(opts: UIFactoryOptions): boolean { if (process.env.SENTRY_INIT_TUI === "0") { @@ -101,18 +93,15 @@ function shouldUseLogging(opts: UIFactoryOptions): boolean { if (!isInteractiveTerminal()) { return true; } - if (!isBunRuntime()) { - return true; - } return false; } /** - * Async factory — picks `InkUI` for interactive runs on the Bun - * binary, otherwise `LoggingUI`. The async form exists because - * instantiating `InkUI` requires a lazy `import("ink")` (the package - * isn't bundled into the npm/Node distribution and would fail to - * resolve if statically imported there). + * Async factory — picks `InkUI` for interactive runs, otherwise + * `LoggingUI`. Works on both the Bun binary and the npm/Node + * distribution (the Ink sidecar is self-contained ESM loaded via + * dynamic `import()`). Falls back to `LoggingUI` if the Ink import + * fails for any reason. * * Callers should treat the return value as an `AsyncDisposable` and * use `await using ui = await getUIAsync(...)` to guarantee teardown diff --git a/src/lib/init/ui/ink-ui.ts b/src/lib/init/ui/ink-ui.ts index c917c40c9..0de16171f 100644 --- a/src/lib/init/ui/ink-ui.ts +++ b/src/lib/init/ui/ink-ui.ts @@ -37,11 +37,13 @@ * process. We close the stream on dispose to release the libuv * handle. * - * **Lazy import.** `ink`, `ink-spinner`, and `react` are all - * dynamically imported by `createInkUI()` so the npm bundle (which - * excludes them from the bundle graph) never sees the imports at - * module-load time. This keeps the `LoggingUI` path cheap to - * instantiate when interactive UI is not needed. + * **Lazy import.** The Ink app sidecar (`ink-app.js`) is a + * self-contained ESM bundle with all deps (ink, react, yoga-layout) + * inlined. It's loaded lazily by `createInkUI()` via dynamic + * `import()` so the `LoggingUI` path stays cheap to instantiate + * when interactive UI is not needed. On the Bun binary the sidecar + * is embedded in `/$bunfs/`; on the npm/Node distribution it ships + * as `dist/ink-app.js` alongside the CJS bundle. */ import { openSync } from "node:fs"; @@ -170,20 +172,19 @@ function severityForStopCode(code: SpinnerExitCode): LogSeverity { * `text-import-plugin` in `script/build.ts` intercepts this during * esbuild: it pre-bundles the .tsx source into self-contained JS * (stripping TypeScript, inlining local deps and npm packages, - * injecting a `createRequire` banner for CJS deps), then marks the - * import external so Bun.compile picks up the resulting .js file. + * injecting a `createRequire` banner for CJS deps). * * Why pre-bundle? Bun's `/$bunfs/` virtual FS uses a JavaScript * parser, not TypeScript — raw .tsx fails on `import { type Foo }`. * The `/$bunfs/` environment also has no `node_modules`, so all * deps (ink, react, local modules) must be inlined. * - * The npm/Node distribution never reaches `createInkUI()` (the - * factory routes there only on the Bun binary because Ink uses - * top-level await that esbuild can't emit in our CJS bundle), so - * the embedded file is unused on Node. We still produce it because - * the static import is unconditional; the bundle.ts cleanup step - * `unlink`s the unused sidecar after bundling. + * For the Bun binary build (ESM), the import is marked external so + * Bun.compile embeds the file. For the npm CJS bundle, the plugin + * emits a virtual module that exports the sidecar filename as a + * string — `createInkUI` then resolves it relative to the bundle + * and loads it via dynamic `import()`. The sidecar ships as + * `dist/ink-app.js` in the npm package. */ // @ts-expect-error: `with { type: "file" }` is Bun-specific and not yet typed in @types/bun import inkAppPath from "./ink-app.tsx" with { type: "file" }; @@ -213,24 +214,32 @@ function openFreshTtyForInk(): ReadStream | null { export async function createInkUI( opts: CreateInkUIOptions = {} ): Promise { - // Import the Ink App sidecar. In compiled binaries the path - // points to `/$bunfs/root/ink-app-xxx.js` (pre-bundled by - // text-import-plugin). In dev mode it's the raw filesystem path - // to `ink-app.tsx`. + // Import the Ink App sidecar. Three runtime contexts: // - // Query-string cache-bust: Bun's module loader caches the - // `with { type: "file" }` import result (a path string) under - // the same specifier key. A bare `await import(inkAppPath)` in - // dev mode returns `{ default: "/abs/path" }` instead of the - // module's actual exports. Appending `?bridge=1` forces a - // distinct cache key so the .tsx is evaluated as a module. + // 1. Bun binary: inkAppPath is "/$bunfs/root/ink-app-xxx.js" + // (embedded by Bun.compile). Import directly — no query string + // (/$bunfs/ doesn't support them). // - // In compiled binaries the path lives under `/$bunfs/` which - // does NOT support query strings (ENOENT). There the cache - // collision doesn't occur because Bun.compile resolves the - // `with { type: "file" }` import differently. - const isEmbedded = inkAppPath.startsWith("/$bunfs/"); - const importPath = isEmbedded ? inkAppPath : `${inkAppPath}?bridge=1`; + // 2. Dev mode (bun run src/bin.ts): inkAppPath is the absolute + // filesystem path to ink-app.tsx. Append ?bridge=1 to bust + // Bun's module cache (otherwise the import returns the path + // string instead of the module's exports). + // + // 3. Node/npm (npx sentry@latest): inkAppPath is a relative path + // like "./ink-app.js" (emitted by text-import-plugin as a + // string literal). Resolve it to an absolute file:// URL using + // import.meta.url so Node's dynamic import() can load the + // self-contained ESM sidecar from the dist/ directory. + let importPath: string; + if (inkAppPath.startsWith("/$bunfs/")) { + importPath = inkAppPath; + } else if (inkAppPath.startsWith("./")) { + // Node/npm bundle — resolve relative to the bundle location + importPath = new URL(inkAppPath, import.meta.url).href; + } else { + // Dev mode — absolute filesystem path, cache-bust for Bun + importPath = `${inkAppPath}?bridge=1`; + } const app = (await import(importPath)) as typeof import("./ink-app.js"); const store = new WizardStore({ diff --git a/src/lib/init/ui/types.ts b/src/lib/init/ui/types.ts index a88b9f7d9..67c376691 100644 --- a/src/lib/init/ui/types.ts +++ b/src/lib/init/ui/types.ts @@ -5,14 +5,14 @@ * provide the actual rendering: * * - `InkUI` — Ink-based React UI. Default for interactive runs on - * the Bun-compiled binary. Ink is pure JS but uses - * top-level await internally, which esbuild can't emit - * in our CJS npm bundle — so the npm/Node distribution - * falls back to `LoggingUI` instead. + * both the Bun binary and the npm/Node distribution. + * The Bun binary embeds Ink via `with { type: "file" }`; + * the npm package ships a self-contained ESM sidecar + * (`dist/ink-app.js`) loaded via dynamic `import()`. * - `LoggingUI` — plain stdout/stderr writes for CI, `--yes`, non-TTY - * environments, the npm/Node distribution, and the - * `--no-tui` escape hatch. Prompts throw — - * non-interactive callers must supply defaults. + * environments, and the `--no-tui` escape hatch. + * Prompts throw — non-interactive callers must supply + * defaults. * * The factory in `factory.ts` picks an implementation per run. * diff --git a/src/lib/init/wizard-runner.ts b/src/lib/init/wizard-runner.ts index f54d60203..f1341721c 100644 --- a/src/lib/init/wizard-runner.ts +++ b/src/lib/init/wizard-runner.ts @@ -8,7 +8,7 @@ * All UI I/O — banners, spinners, logs, prompts, outro — flows through * a single `WizardUI` instance constructed by `getUI()`. The runner * itself is implementation-agnostic: it works the same against - * `LoggingUI` (CI / npm) and `InkUI` (interactive Bun binary). + * `LoggingUI` (CI / `--yes`) and `InkUI` (interactive terminal). */ import { randomBytes } from "node:crypto"; @@ -487,8 +487,7 @@ export async function runWizard(initialOptions: WizardOptions): Promise { // Construct the UI once for the entire run; tear down on every exit // path via `await using`. The factory picks `InkUI` for interactive - // runs on the Bun binary and `LoggingUI` everywhere else (CI, - // `--yes`, `--no-tui`, npm/Node distribution). + // runs and `LoggingUI` for CI / `--yes` / `--no-tui`. const initialWelcome = yes || dryRun ? undefined : buildWelcomeOptions(); await using ui = await getUIAsync({ yes, diff --git a/test/lib/init/wizard-runner.test.ts b/test/lib/init/wizard-runner.test.ts index e39d975b3..70b882a75 100644 --- a/test/lib/init/wizard-runner.test.ts +++ b/test/lib/init/wizard-runner.test.ts @@ -775,6 +775,29 @@ describe("runWizard", () => { expect(spinnerMock.stop).toHaveBeenCalledWith("Using existing project"); }); + + test("shows --yes hint when LoggingUI prompt fails", async () => { + const { LoggingUIPromptError } = await import( + "../../../src/lib/init/ui/logging-ui.js" + ); + const { ui } = createMockUI(); + const failingUI: WizardUI = { + ...ui, + spinner: () => spinnerMock, + select: () => + Promise.reject( + new LoggingUIPromptError( + "select", + "This is experimental and will modify files" + ) + ), + }; + getUISpy.mockResolvedValue(failingUI); + + await expect( + forceStdinTty(() => runWizard(makeOptions({ yes: false }))) + ).rejects.toThrow("Run with --yes for non-interactive mode."); + }); }); describe("runWizard — MastraClient lifecycle", () => {