| 1 | # Plan 023: Decompose the god functions in `export.ts` and `cli.ts` |
| 2 | |
| 3 | > **Executor instructions**: This is a **behavior-preserving refactor** of two |
| 4 | > critical, high-churn paths. Do it in small, verifiable steps; never change what |
| 5 | > the code does, only how it's organized. Run the full test suite after each |
| 6 | > step. Honor the STOP conditions. When done, update the status row in |
| 7 | > `plans/README.md`. |
| 8 | > |
| 9 | > **Drift check (run first)**: `git diff --stat c63cb120..HEAD -- packages/slidev/node/commands/export.ts packages/slidev/node/cli.ts` |
| 10 | > On a mismatch with the excerpts below, treat it as a STOP condition. |
| 11 | |
| 12 | ## Status |
| 13 | |
| 14 | - **Priority**: P3 |
| 15 | - **Effort**: L |
| 16 | - **Risk**: MED |
| 17 | - **Depends on**: **plan 022** (export characterization tests) MUST land first; |
| 18 | benefits from 007 (browser teardown) and 013 already applied |
| 19 | - **Category**: tech-debt |
| 20 | - **Planned at**: commit `c63cb120`, 2026-07-10 |
| 21 | |
| 22 | ## Why this matters |
| 23 | |
| 24 | Two functions concentrate risk and resist testing: |
| 25 | |
| 26 | - `exportSlides` (`export.ts:167-572`) is a ~400-line function wrapping 13 nested |
| 27 | closures (`go`, `getSlidesIndex`, `genPageWithClicks`, `genPagePdf*`, |
| 28 | `genPagePng*`, `genPageMd`, `genPagePptx`, `addPdfMetadata`, `addTocToPdf`) that |
| 29 | share mutable `output`/`page`/`progress` via closure. The per-format exporters |
| 30 | can't be unit-tested or reused independently. |
| 31 | - The default serve command handler (`cli.ts:114-340`) is a ~226-line closure |
| 32 | holding `initServer`, `restartServer`, tunnel/QR/open helpers, the `SHORTCUTS` |
| 33 | table, `bindShortcut`, and the chokidar watcher — mixing CLI wiring, server |
| 34 | lifecycle, TTY shortcuts, and file-watching. |
| 35 | |
| 36 | Both are frequently edited (git churn), so the coupling compounds maintenance |
| 37 | cost and risk. |
| 38 | |
| 39 | ## Current state |
| 40 | |
| 41 | - `export.ts`: `exportSlides(options)` opens a browser/context/page, dispatches on |
| 42 | `format`, and all `gen*` helpers are nested functions closing over `page`, |
| 43 | `output`, `progress`, `pages`, `width`, `height`, etc. (see `export.ts:167-572`). |
| 44 | - `cli.ts`: the `default` yargs command handler (`:114-340`) defines server |
| 45 | lifecycle + shortcuts + watcher inline. |
| 46 | - Safety net: after plan 022, `export.ts` has characterization tests for |
| 47 | `getExportOptions` + outline/range helpers. There are **no** tests for the serve |
| 48 | handler (it's interactive), so its refactor must be especially conservative. |
| 49 | |
| 50 | ## Commands you will need |
| 51 | |
| 52 | | Purpose | Command | Expected | |
| 53 | |---------|---------|----------| |
| 54 | | Install | `pnpm install` | exit 0 | |
| 55 | | Build | `pnpm build` | exit 0 | |
| 56 | | Test | `pnpm test` | all pass (incl. plan 022's) | |
| 57 | | Typecheck | `pnpm typecheck` | exit 0 | |
| 58 | | Lint | `pnpm lint` | exit 0 | |
| 59 | |
| 60 | ## Scope |
| 61 | |
| 62 | **In scope**: |
| 63 | - `packages/slidev/node/commands/export.ts` (extract per-format exporters behind |
| 64 | an explicit context object) |
| 65 | - optionally new files under `packages/slidev/node/commands/export/` for the |
| 66 | extracted exporters |
| 67 | - `packages/slidev/node/cli.ts` (lift serve-handler helpers into a module with |
| 68 | injected deps) — **only if** it can be done without behavior change |
| 69 | |
| 70 | **Out of scope**: |
| 71 | - Any change to export output, CLI flags, shortcut keys, or server behavior. |
| 72 | - The public signatures of `exportSlides`/`exportNotes`/`getExportOptions` (keep |
| 73 | them stable — they're imported by `build.ts` and `cli.ts`). |
| 74 | |
| 75 | ## Git workflow |
| 76 | |
| 77 | - Branch: `refactor/decompose-export-serve`. |
| 78 | - One commit per extracted unit; conventional: `refactor(export): extract PngExporter`, etc. |
| 79 | - Do NOT push/PR unless instructed. |
| 80 | |
| 81 | ## Steps |
| 82 | |
| 83 | > Do the **export** decomposition first (it has tests). Treat the **serve** |
| 84 | > handler as a second, optional phase and STOP for confirmation before starting it. |
| 85 | |
| 86 | ### Step 1: Introduce an explicit export context |
| 87 | |
| 88 | Define an `ExportContext` object holding what the closures currently capture |
| 89 | (`page`, `output`, `progress`, `pages`, `width`, `height`, `withClicks`, `range`, |
| 90 | flags). Change `exportSlides` to build it once and pass it to the (still-nested, |
| 91 | for now) helpers as a parameter instead of relying on closure capture. |
| 92 | |
| 93 | **Verify**: `pnpm build && pnpm test` → green; export snapshots unchanged. |
| 94 | |
| 95 | ### Step 2: Extract per-format exporters |
| 96 | |
| 97 | Move `genPagePdf*`/`genPagePng*`/`genPageMd`/`genPagePptx` into standalone |
| 98 | functions (e.g. `PdfExporter(ctx)`, `PngExporter(ctx)`, …) that take the |
| 99 | `ExportContext`. `exportSlides` becomes: build browser/context/page → build ctx → |
| 100 | dispatch to the chosen exporter → (try/finally close, from plan 007). Keep |
| 101 | `addPdfMetadata`/`addTocToPdf`/`makeOutline`/`getSlidesIndex` as helpers the |
| 102 | exporters call. |
| 103 | |
| 104 | **Verify after each extraction**: `pnpm build && pnpm test && pnpm typecheck` → |
| 105 | green; no export snapshot/behavior change. |
| 106 | |
| 107 | ### Step 3 (optional, STOP-gated): lift the serve handler helpers |
| 108 | |
| 109 | Only after Step 2 and operator confirmation: extract `initServer`/`restartServer`/ |
| 110 | tunnel-QR-open/`bindShortcut`/watcher into a `SlidevDevServerController` module |
| 111 | with injected dependencies, leaving `cli.ts` to wire args → controller. Because |
| 112 | there are no automated tests here, do this in the smallest possible commits and |
| 113 | verify manually with `pnpm demo:dev` (server starts, restart on config change, |
| 114 | shortcuts `o`/`e`/`q`, watcher reload). |
| 115 | |
| 116 | **Verify**: `pnpm build && pnpm typecheck && pnpm lint` green; manual serve smoke |
| 117 | passes. |
| 118 | |
| 119 | ## Test plan |
| 120 | |
| 121 | - Rely on plan 022's characterization tests to prove the export refactor is |
| 122 | behavior-preserving — run `pnpm test` after every extraction. |
| 123 | - Add per-exporter unit tests where a unit becomes independently testable without |
| 124 | Playwright (e.g. filename/range logic). |
| 125 | - Serve handler: manual smoke only (no harness); keep commits tiny. |
| 126 | |
| 127 | ## Done criteria |
| 128 | |
| 129 | - [ ] `exportSlides` dispatches to standalone per-format exporters via an explicit context (no shared-closure mutable state for the format logic) |
| 130 | - [ ] Public signatures of `exportSlides`/`exportNotes`/`getExportOptions` unchanged |
| 131 | - [ ] All plan 022 tests still pass; export output/snapshots unchanged |
| 132 | - [ ] (If Step 3 done) serve behavior manually verified unchanged |
| 133 | - [ ] `pnpm build && pnpm typecheck && pnpm lint && pnpm test` pass |
| 134 | - [ ] `plans/README.md` status row updated |
| 135 | |
| 136 | ## STOP conditions |
| 137 | |
| 138 | Stop and report if: |
| 139 | |
| 140 | - Plan 022's tests are **not** in place — do not refactor export without the net. |
| 141 | - Any export snapshot/output changes — that means the refactor altered behavior; |
| 142 | revert the step. |
| 143 | - The serve-handler extraction (Step 3) starts changing observable behavior or |
| 144 | balloons in scope — stop and keep only the export decomposition. |
| 145 | |
| 146 | ## Maintenance notes |
| 147 | |
| 148 | - After this, adding a new export format is a new exporter file, not another |
| 149 | nested closure. |
| 150 | - Reviewer: diff should be almost entirely code movement; scrutinize any line |
| 151 | that isn't a pure move. |
| 152 |