Skip to content

fix(exporters): compile JSX before export (N0) + semantic export E2E (N1) - #42

Open
stephschofield wants to merge 3 commits into
mainfrom
worktree-n0-export-contract
Open

fix(exporters): compile JSX before export (N0) + semantic export E2E (N1)#42
stephschofield wants to merge 3 commits into
mainfrom
worktree-n0-export-contract

Conversation

@stephschofield

@stephschofield stephschofield commented Jul 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

The bug

Every export format shipped uncompiled JSX. store.ts:exportActive read previewHtml — which is not HTML, but the agent's raw JSX module source — and handed it to the exporters as htmlContent. ensureDocumentShell saw no doctype, wrapped the bare JSX in a <body>, and wrote a file containing literal JSX text with no React and no Babel. It opened to a blank page. All five formats (html/pdf/pptx/zip/markdown) inherited this.

The three other consumers of the artifact — PreviewPane, done-verify, DesignCardPreview — all compile via buildSrcdoc first. Export was the only path that did not.

Root cause was naming: nothing in the type system said whether htmlContent wanted JSX or HTML.

N0 — make the contract checkable

  • ArtifactSource = {kind:'jsx'|'html'; source:string} + requireRenderedHtml(), which throws rather than exporting raw JSX.
  • Rename htmlContent → artifactSource (IPC/preload) and → renderedHtml (exporter internals), end to end.
  • Compile once via buildSrcdoc in exportActive, before dispatch.
  • Guards at both boundaries: parseRequest rejects kind:'jsx'; exportArtifact re-checks at dispatch.
  • ExportRequest.artifactSource narrowed to kind:'html', so "already compiled" is a type-level guarantee rather than a convention.
  • New EXPORTER_COMPILE_FAILED code + en/pt-BR/zh-CN strings; compile failures surface as that code instead of a generic export-failed toast.

Two further bugs found while writing the tests

  1. Double doctype. buildHtmlDocument prepends its banner comment before the doctype, so ensureDocumentShell failed to recognise its own output and wrapped a second doctype on any re-export. Fixed by sniffing past leading comments.
  2. Silently empty markdown. Markdown export of a client-rendered document produced a 34-byte file — YAML frontmatter only (convertBody strips <head> and every <script>, leaving nothing) — and reported success. Now fails loudly. Static prerendering is N6.

N1 — an E2E that can actually catch this

The originally-specified assertion was "the exported file has an HTML doctype." That is vacuous: the doctype is prepended unconditionally, so it passed on the broken output. The test as specified would have certified the bug it existed to catch.

This asserts the property a user cares about: the file renders. It drives the real export IPC (only the save dialog is stubbed), opens the written file in an Electron BrowserWindow with all network egress blocked at the session level, and requires #root gained children and the artifact's content is on screen.

Verified by mutation: reverting the compile step makes this test fail on the #root wait (30s timeout, no children — the blank page); restoring it makes it pass. A regression test that cannot catch the regression is worse than none.

The run surfaced two genuinely blocked requests — the Tailwind CDN tag and the hardcoded Google Fonts stylesheet. Both are known (plan A5: en.json:629 already claims HTML export is "single self-contained", which is false today). Resolving them is N6, so they are pinned explicitly with a message telling the next person to tighten the bound to zero once N6 lands, rather than hidden behind a blanket filter.

Test plan

  • pnpm test — 2292 passing, 0 failing
  • pnpm typecheck — 10/10 packages
  • pnpm lint — clean
  • pnpm build — now completes (AppImage + deb; rpm skipped with a clear message)
  • N1 E2E passes; fails on mutated (pre-N0) code
  • html.ts had zero coverage — which is how this shipped unnoticed. Adds html.test.ts (26 cases) + export-pipeline.test.ts (green path; lives in apps/desktop because packages/exporters deliberately does not depend on @atv-design/runtime)
  • Manual: export a real design to each of the five formats — done, against a packaged build
html      3317715 bytes  compiled runtime present
pdf         37595 bytes  valid pdf container
pptx        47518 bytes  valid pptx container
zip        719159 bytes  valid zip container
markdown        0 bytes  correctly refused (client-rendered; N6 will fix)

Automated as e2e/export-all-formats.spec.ts, which drives the real export IPC (only the save dialog is stubbed) and checks each written file for format-appropriate evidence — magic bytes for the PDF/PPTX/ZIP containers, compiled runtime for the HTML.

Markdown refusing is the intended N0 behaviour, not a gap: the compiled artifact renders on the client, so no static HTML survives convertBody to convert. A companion test exports a static HTML artifact to markdown successfully, proving the guard rejects client-rendered documents specifically rather than disabling markdown export wholesale.

Getting there surfaced two more real bugs

PDF and PPTX export were broken in every packaged build. pnpm --prod deploy prunes by apps/desktop/package.json, and puppeteer-core / pptxgenjs were only reachable through @atv-design/exporters — a devDependency. Listing app.asar confirmed 0 entries for both, so exporting to PDF or PPTX from an installed build failed with ERR_MODULE_NOT_FOUND. zip-lib survived only by luck: it was already a direct dep. Never visible in dev, where the lazy imports resolve against the workspace node_modules. Both are now declared, with a test pinning all three (verified to fail when one is removed).

pnpm build died on any host without rpmbuild. electron-builder shells out to external tools for Linux installers (rpm→rpmbuild, deb→dpkg) and bundles neither, so the build failed after compiling, rebuilding native modules, and downloading Electron — taking all of pnpm build with it. Default builds now drop only what the host genuinely can't build and log why; AppImage always survives; an explicit --linux <target>/--dir is still a loud error rather than a silent downgrade. CI images have both tools, so release builds are unaffected.

One notable detail: the target list must be passed as --linux a b, not repeated --config.linux.target= flags — that key is a scalar, so each occurrence overwrites the last. My first cut silently dropped the AppImage. There's a regression test pinning it.

Pre-existing, not from this PR

6 design-system-* E2E screenshot-baseline failures. Confirmed identical on base commit bebb02d; those specs contain zero references to export.

Follow-ups (already scoped in the plan)

  • N6 — html.ts Tier 2: inline fonts, precompile away the Tailwind CDN, strip the overlay/tweaks harness. Makes the "self-contained" string true, unblocks markdown/pptx static content, and drops the E2E's blocked-request bound to zero.

@stephschofield
fix(exporters): compile JSX before export; rename htmlContent -> arti…
9e7913b 
…factSource

Every export format shipped uncompiled JSX. `store.ts:exportActive` read
`previewHtml` — which is not HTML, but the agent's raw JSX module source —
and passed it straight to the exporters as `htmlContent`. `ensureDocumentShell`
saw no doctype, wrapped the bare JSX in a <body>, and wrote a file containing
literal JSX text with no React and no Babel. It opened to a blank page. All
five formats (html/pdf/pptx/zip/markdown) inherited this.

The three other consumers of the artifact (PreviewPane, done-verify,
DesignCardPreview) all compile via `buildSrcdoc` first. Export was the only
path that did not.

Root cause was naming: nothing in the type system said whether `htmlContent`
wanted JSX or HTML. This makes the contract checkable rather than implicit:

- Add `ArtifactSource = {kind:'jsx'|'html'; source:string}` and
  `requireRenderedHtml()`, which throws instead of exporting raw JSX.
- Rename htmlContent -> artifactSource (IPC/preload) and -> renderedHtml
  (exporter internals) end-to-end.
- Compile once via `buildSrcdoc` in `exportActive` before dispatch.
- Guard at both boundaries: `parseRequest` rejects kind:'jsx', and
  `exportArtifact` re-checks at dispatch.
- Narrow `ExportRequest.artifactSource` to kind:'html' so "already compiled"
  is a type-level guarantee, not a convention.
- Add EXPORTER_COMPILE_FAILED + en/pt-BR/zh-CN strings; surface compile
  failures as that code rather than a generic export-failed toast.

Two further bugs found while writing the tests:

- `buildHtmlDocument` prepends its banner comment before the doctype, so
  `ensureDocumentShell` failed to recognise its own output and wrapped a
  second doctype around it on any re-export. Fixed by sniffing past leading
  comments.
- Markdown export of a client-rendered document produced a 34-byte file
  containing only YAML frontmatter — `convertBody` strips <head> and every
  <script>, leaving nothing. It reported success. Now fails loudly; static
  prerendering is N6.

Tests: html.ts had zero coverage, which is how this shipped unnoticed.
Adds html.test.ts (26 cases: document-shell characterization, the JSX
regression net, comment/idempotency edge cases) and export-pipeline.test.ts
(the compile-then-export green path, which lives in apps/desktop because
packages/exporters deliberately does not depend on @atv-design/runtime).

2280 tests pass; typecheck and lint clean.
@stephschofield
test(e2e): assert exported files actually render, not just that they …
53bc01a 
…parse

N1. The originally-specified assertion for this test was "the exported file
has an HTML doctype". That assertion is vacuous — `buildHtmlDocument` prepends
a doctype unconditionally, so it passed on the pre-N0 output: a file of
literal, unexecutable JSX that opened to a blank page. The test as specified
would have certified the bug it existed to catch.

This asserts the property a user actually cares about: the file renders. It
drives the real export IPC (the same path the Export menu uses, with only the
save dialog stubbed), opens the written file in an Electron BrowserWindow
with all network egress blocked at the session level, and requires that
`#root` gained children and the artifact's own content is on screen.

Network blocking is load-bearing. `buildSrcdoc` inlines React, ReactDOM, and
Babel, so a correct export renders fully offline; if a refactor swaps those
inlines for CDN tags, this fails.

Verified by mutation: reverting the compile step in `exportActive` to the
pre-N0 behaviour makes this test fail on the `#root` wait (30s timeout, no
children — the blank page), and restoring it makes it pass. A regression test
that cannot catch the regression is worse than none.

The run surfaced two genuinely blocked requests: the Tailwind CDN tag and the
hardcoded Google Fonts stylesheet. Both are known (plan A5: `en.json:629`
already claims HTML export is "single self-contained" and is false today).
Resolving them is N6, so they are pinned explicitly with a message telling the
next person to tighten the bound to zero once N6 lands, rather than hidden
behind a blanket filter.

Playwright's Electron driver cannot create browser contexts
(`Target.createTarget` unsupported), hence the BrowserWindow + `webRequest`
approach rather than `page.route`.
@stephschofield
fix(build): ship puppeteer-core + pptxgenjs; degrade linux targets gr…
42342e5 
…acefully

Unblocks the manual export verification in PR #42, and fixes two real bugs
found while doing it.

1. PDF and PPTX export were broken in every packaged build.

`pnpm --prod deploy` prunes by apps/desktop/package.json. `puppeteer-core`
and `pptxgenjs` were only reachable through `@atv-design/exporters`, which is
a devDependency, so neither ever made it into the shipped asar — confirmed by
listing app.asar: 0 entries for both. Exporting to PDF or PPTX from an
installed build failed at runtime with ERR_MODULE_NOT_FOUND. `zip-lib`
survived purely because it happened to be listed as a direct dep already.

Both are now declared alongside it, and a test asserts all three stay there
(verified to fail when one is removed). This never showed up in dev because
the lazy imports resolve fine against the workspace node_modules.

2. `pnpm build` died on any host without rpmbuild.

electron-builder shells out to external tools for Linux installers — rpm
needs `rpmbuild`, deb needs `dpkg` — and neither is bundled. On a host
missing one, the build failed after compiling, rebuilding native modules, and
downloading Electron, taking all of `pnpm build` down with it. The failure is
environmental, not a defect, but it blocked every local task needing a
runnable binary.

Default builds now drop only the targets the host genuinely cannot build and
log why. AppImage has no external dependency so it always survives. An
explicit `--linux <target>` or `--dir` is left alone, so asking for an
unbuildable target is still a loud error rather than a silent downgrade. CI
images have both tools, so release builds are unaffected.

Note the target list is passed as `--linux a b`, NOT repeated
`--config.linux.target=` flags — that key is a scalar, so each occurrence
overwrote the last and only the final target survived. The first cut of this
silently dropped the AppImage; there is a regression test pinning it.

3. Adds e2e/export-all-formats.spec.ts.

Drives the real export IPC for all five formats and checks each written file
for format-appropriate evidence (PDF/PPTX/ZIP magic bytes, compiled runtime
in the HTML). Result against a packaged build:

  html      3317715 bytes  compiled runtime present
  pdf         37595 bytes  valid pdf container
  pptx        47518 bytes  valid pptx container
  zip        719159 bytes  valid zip container
  markdown        0 bytes  correctly refused (client-rendered; N6 will fix)

Markdown refusing is the intended N0 behaviour, not a gap: the compiled
artifact renders on the client, so there is no static HTML left to convert.
A second test exports a static HTML artifact to markdown successfully,
proving the guard rejects client-rendered documents specifically rather than
disabling markdown export wholesale.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

area:build area:desktop area:exporters

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@stephschofield