feat(export): add reliable standalone HTML export - #6855
Conversation
Bundle project-local HTML, CSS, JavaScript, images, fonts, workers, and iframe dependencies into one offline-safe file. Expose the same contract through the web download flow and od CLI, with structured failures and bounded output assembly.
lefarcen
added
needs-validation
|
🧪 This PR has changes that need a manual QA pass before merge — please hold off self-merging for now; we will loop QA in once it is merge-ready. |
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
I found two correctness blockers in the standalone bundler: relative URLs ignore HTML base-URL semantics, and TypeScript/JSX sources are accepted but emitted without compilation. Both can produce a successful download that fails when opened offline. I also found a resource-fanout guard that can be bypassed by valid unquoted HTML attributes.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.| } | ||
| const joined = pathname.startsWith('/') | ||
| ? decoded.replace(/^\/+/, '') | ||
| : path.posix.join(path.posix.dirname(ownerPath), decoded); |
There was a problem hiding this comment.
Blocking — base URL resolution: This always joins a relative reference against dirname(ownerPath), but the browser applies the document's <base href=...> before resolving it. For example, <base href=assets/> with <img src=logo.png> is read as pages/logo.png here instead of assets/logo.png; the export can therefore inline the wrong file or report a missing dependency, while the untouched <base> leaves other relative URLs pointing at the wrong place after download. Please parse the effective base URL for each document, resolve all references against it, and rewrite or remove the base element in the standalone output (or return a structured unsupported-base error).
| try { | ||
| ast = parse(source, { | ||
| sourceType: mode === 'classic' ? 'unambiguous' : 'module', | ||
| plugins: ['dynamicImport', 'importAttributes', 'importMeta', 'jsx', 'topLevelAwait', 'typescript'], |
There was a problem hiding this comment.
Blocking — source compilation: This parser configuration accepts TypeScript and JSX syntax, but rewriteJavaScript only rewrites URL/import literals and then emits the original source into a data URL. A Vite-style <script type=module src=/src/main.tsx> (when no built dist entry is available) therefore produces a 200 export containing const x: number or JSX, which the browser rejects at runtime. Please transpile TS/JSX before embedding or reject non-runtime source with a structured invalid-source/422 response instead of returning a broken artifact.
| return /\srel\s*=\s*(['"])[^'"]*\bstylesheet\b[^'"]*\1/iu.test(tag) | ||
| && /\shref\s*=\s*(['"])[^'"]+\1/iu.test(tag); | ||
| }).length; | ||
| const scripts = [...html.matchAll(/<script\b[^>]*\bsrc\s*=\s*(['"])[^'"]+\1[^>]*>/giu)].length; |
There was a problem hiding this comment.
Non-blocking — candidate limit accounting: The preflight guard only matches quoted src/href values, while Cheerio and the bundling loops also process valid unquoted attributes. For example, 501 <script src=a.js></script> entries bypass firstLevelCandidates and are all read and inlined, so the documented 500-resource fan-out guard can be defeated. Please count candidates with the same parser/attribute semantics as bundleDocument (or extend this matcher to accept unquoted values) before walking the graph.
Visual regression reviewHead: 10 changed · 39 unchanged · 0 new without baseline · 0 failed Changed cases
Unchanged cases
Visual diff is advisory only and does not block merging. |
hihijack111
added
backport release/v0.19.1
Resolve document assets against local base href values, reject browser-invalid TypeScript and JSX sources with a structured 422, and count unquoted first-level resources before graph traversal. Validated with daemon tests, workspace typecheck, and guard.
|
Addressed the three Looper findings in 439aba0: document resources now honor the first local base href and base hrefs are stripped from the standalone output; TypeScript/JSX is rejected as structured invalid-source (HTTP 422) instead of emitted unchanged; and the first-level resource guard now counts quoted and unquoted attributes with the bundler parser. Added red-to-green unit and route coverage; daemon tests, workspace typecheck, and guard pass. |
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
@app/open-design-crew
I found two blocking correctness issues in the standalone bundler: valid external data scripts are parsed as JavaScript, and CSS URL scanning treats quoted or comment text as dependencies. Both can make an otherwise valid project return a 422 instead of producing an offline HTML file.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.| await this.ensureModule(local.projectPath, [...chain, local.projectPath], modulePaths); | ||
| body = `import ${JSON.stringify(moduleSpecifier(local.projectPath))};`; | ||
| } else { | ||
| body = await this.rewriteJavaScript( |
There was a problem hiding this comment.
Blocking — non-JavaScript external scripts are treated as JavaScript
Every external script whose type is not exactly module reaches rewriteJavaScript at this line. A valid <script type="application/ld+json" src="schema.json"></script> (and other data-script types) is therefore parsed as JavaScript; JSON-LD keys such as "@context" make Babel reject it, so the endpoint returns invalid-source/422 instead of embedding the data script. Gate rewriting on JavaScript/module MIME types (and keep data scripts as escaped text or a correctly typed data URL), then add a JSON-LD external-script fixture.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.| for (const match of value.matchAll(URL_FUNCTION_RE)) { | ||
| const reference = match[2] ?? match[3]; | ||
| if (!reference || match.index === undefined) continue; | ||
| const rewritten = await this.referenceToDataUrl(ownerPath, reference.trim(), chain); |
There was a problem hiding this comment.
Blocking — URL scanning is not CSS-token aware
rewriteCssUrls applies a regex to the raw declaration/style-attribute text and immediately resolves every match as a project dependency. Valid text such as style="content:'url(missing.png)'" (or a URL-looking CSS comment) is consequently mistaken for a dependency and makes export fail with missing-local-dependency; the browser would keep it as text. Rewrite only actual CSS url() tokens by using a CSS value/token parser or a scanner that skips strings and comments, and add fixtures for quoted strings, comments, custom properties, and real URLs so valid HTML does not turn into a 422.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Preserve non-JavaScript script payloads as data blocks and scan CSS values without treating strings or comments as project dependencies. Validated with focused and full daemon tests, workspace typecheck, and guard.
|
Addressed both new Looper blockers in f616306. Non-JavaScript script types such as application/ld+json are now embedded as escaped data blocks without Babel parsing, while classic/module scripts keep their existing rewrite behavior. CSS URL discovery now skips quoted strings and comments while still rewriting real url() tokens, including those stored in custom properties. Added red-to-green fixtures; 62 focused/route tests, the full daemon suite, guard, and workspace typecheck pass. |
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
@app/open-design-crew
The standalone export implementation and its follow-up hardening are thoughtfully covered by focused tests. I ran the daemon bundler/route suite (72 tests) and the web export/viewer suite (371 tests); both pass, but two correctness blockers remain in the changed bundling path.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Location: apps/daemon/src/import-export-routes.ts RIGHT line 1712
Non-blocking — Vite dist lookup swallows all failures.
This catch treats permission errors, I/O failures, and unexpected parser/read errors exactly like a missing dist/index.html, silently falling back to the dev HTML. That can export a stale/unbuilt source or turn an operational failure into a misleading invalid-source response. Catch only an explicit not-found condition and propagate other errors so the route's structured error handling reports the real failure; cover the distinction with a route test.
Inline comment could not be anchored: anchor_outside_complete_diff
| chain, | ||
| documentStack, | ||
| ); | ||
| return this.checkedDataUrl('text/html;charset=utf-8', Buffer.from(nested), chain); |
There was a problem hiding this comment.
Blocking — non-HTML iframe resources are emitted as HTML.
bundleDocumentUrl unconditionally decodes every local iframe as UTF-8 HTML and wraps it in data:text/html. A valid <iframe src="preview.png"> or <iframe src="report.pdf"> therefore becomes an HTML document containing binary text, so the endpoint returns a successful export while the iframe is broken offline. Inspect loaded.mime before recursively bundling; for non-HTML dependencies emit checkedDataUrl(loaded.mime, loaded.buffer, ...) directly (or return a structured unsupported-resource error), and add an image/PDF iframe fixture.
| }); | ||
| wholeNodeReplacements.add(node); | ||
| } else { | ||
| await this.rewriteAttribute(html, node, 'href', documentOwnerPath, chain, replacements); |
There was a problem hiding this comment.
Blocking — navigation metadata is treated as a required asset.
The non-stylesheet branch rewrites every local <link> href through referenceToDataUrl, so ordinary metadata/navigation links such as <link rel="canonical" href="/products/demo"> or <link rel="alternate" href="next.html"> must exist as project files. If they do not, a valid document fails with missing-local-dependency/422 even though those links are not runtime dependencies. Restrict this rewrite to resource relations such as icon, manifest, preload, and modulepreload, and leave navigation/metadata relations untouched (or classify them as external); add a regression fixture for a missing canonical URL.
Preserve non-HTML iframe MIME types, inline only resource-bearing link relations, and propagate unexpected Vite dist read failures instead of silently falling back. Validated with focused export tests, the full daemon suite, guard, and workspace typecheck.
|
Addressed the latest review findings in
Added red-to-green coverage for image/PDF iframes, resource vs. metadata links, and the Vite dist read-error path. I also audited the adjacent resource classification, MIME, cache/limit, and error-handling paths for the same classes of issue; no further gaps were found. Validation: focused export tests (65 passed), full daemon suite (8,424 passed / 5 skipped), Please review the latest head. The existing human QA hold remains in place; no merge action was taken. |
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
@app/open-design-crew
The standalone bundler is well covered by the focused daemon/web tests, package typechecks, guard, and the offline browser witness. I found two correctness regressions in the changed export paths; the inline comments describe cases that either always fail or change page appearance, so I’m requesting changes before approval.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.| res, | ||
| 409, | ||
| 'CONFLICT', | ||
| 'standalone HTML cannot export a historical entry with current project dependencies', |
There was a problem hiding this comment.
This hard-rejects every non-empty versionId, but the existing FileViewer still exposes “Export as standalone HTML” for historical versions: runProjectVersionExport puts the selected version id into the context and triggerHtmlExport forwards it here. Before this PR, that action fetched the versioned source and fell back to the selected version’s context.content; now every historical click ends in this 409 and no file is downloaded. That is a user-visible regression in an existing export action. Either bundle a versioned dependency snapshot, or hide/disable this menu item for historical versions (and test the disabled/hidden state) until snapshots are available.
| replacements.push({ | ||
| start: location.startOffset, | ||
| end: location.endOffset, | ||
| value: `<style data-od-inline-asset="${escapeHtmlAttribute(href)}"${kept}>${escapeStyleBody(css)}</style>`, |
There was a problem hiding this comment.
The replacement drops stylesheet link semantics for valid disabled/alternate links. For <link rel="stylesheet" disabled>, this emits <style ... disabled>, but disabled is not honored on a <style> element, so CSS becomes active; for rel="alternate stylesheet", removing rel also turns an opt-in alternate theme into active CSS. The exported page can therefore look different from the source. Keep disabled/alternate links as links with an embedded data URL (or otherwise preserve their enabled/alternate state), and add fixtures for both states before replacing local stylesheets.
Hide standalone HTML export for historical versions until dependency snapshots are available, avoiding an action that always fails. Keep disabled and alternate stylesheets as embedded link elements so standalone export preserves activation semantics. Validated with focused red/green coverage, the full web suite, pnpm guard, and pnpm typecheck.
|
Addressed both findings in e39191d:
Added regression coverage for both cases. Validation passed: |
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
@app/open-design-crew The standalone export work is backed by focused daemon/web tests, workspace guard and typecheck, and passing PR checks. I found three blocking cases where the endpoint can return a successful download that is not actually usable offline: reused module closures in nested documents, non-iframe nested HTML, and preserved CSP policies blocking injected resources. The details and fixes are in the inline comments; requesting changes so the new reliability guarantee holds across these inputs.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.| private async ensureModule(projectPath: string, chain: string[], modulePaths: Set<string>): Promise<void> { | ||
| modulePaths.add(projectPath); | ||
| const state = this.moduleState.get(projectPath); | ||
| if (state === 'complete' || state === 'processing') return; |
There was a problem hiding this comment.
Blocking — cached modules drop transitive imports in nested documents. moduleState is shared by the bundler, but each bundleDocument builds a fresh modulePaths set for its own import map. If the entry first completes main.js -> motion.js, a nested document that also loads main.js returns here without adding motion.js; the nested map only contains main.js while its code still imports od-project:/scripts/motion.js. The iframe then fails to load offline. Please retain the complete transitive module closure when a cached module is reused (or reconstruct it from a dependency graph), and add a fixture that reuses a module in the entry and an iframe. 🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.
| pushAttributeReplacement(html, node, 'style', rewritten, replacements); | ||
| } | ||
|
|
||
| for (const node of $('iframe[src]').toArray() as any[]) { |
There was a problem hiding this comment.
Blocking — nested HTML is only recursively bundled for iframe[src]. object data and embed src go through referenceToDataUrl, which base64-encodes raw HTML, while iframe srcdoc is never visited. For an object or srcdoc containing relative image, script, or CSS references, the downloaded data/file URL cannot resolve those project paths, so the export can succeed while the nested page is broken. Please route HTML MIME object/embed through bundleDocumentUrl and bundle srcdoc against the parent document base (or reject these variants with a structured unsupported-resource error), with regression fixtures and an offline browser check. 🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.
| ); | ||
| } | ||
|
|
||
| let output = applyReplacements(html, replacements.values); |
There was a problem hiding this comment.
Blocking — preserved CSP can disable injected resources. The bundler leaves source Content-Security-Policy meta tags untouched while replacing external scripts and styles with inline/data URLs and injecting a data-valued import map. A page carrying script-src self or style-src self therefore returns 200 but refuses the generated module, import-map, and CSS in the saved file, so it is not offline-functional. Please remove or rewrite conflicting meta CSP for standalone output, or return a structured unsupported-CSP error, and add a fixture/browser assertion. 🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.
AmyShang-alt
added
validated
|
@app/open-design-crew Thanks for the contribution. I completed QA validation for this PR. QA Acceptance Record Scope:
Verified:
Not verified:
Risks / notes:
Conclusion:
|
|
Successfully created backport PR for |
Siri-Ray
removed
the
backport release/v0.19.1
5 participants
Why
Use case: Plane OPEND-1758 reports that downloaded HTML should remain usable after it is moved away from the project directory: https://plane.powerformer.net/open-design/browse/OPEND-1758/
Pain: The existing export only inlined first-level CSS and scripts, so nested images, fonts, modules, workers, and relative URLs could break when users opened the file locally.
What users will see
Downloading HTML now produces one offline-safe file whose project-local images, styles, fonts, modules, workers, and nested documents remain usable. Missing or oversized dependencies produce a clear error instead of a silently broken download. The same format is available through
od export --format html.Surface area
apps/weborapps/desktop(including Electron menu bar)odsubcommand or flag, newtools-dev/tools-packflag, or newOD_*env var/api/*endpoint, new SSE event, or changed shape inpackages/contractsskills/,design-systems/,design-templates/, orcraft/, or change to the skills protocolTRANSLATIONS.mdfor the locale workflow)package.json(dependenciesordevDependencies); workspace-packagepackage.jsonfiles are out of scope. Include a paragraph on what we get vs. what bytes we ship (seeCONTRIBUTING.md→ Code style)Screenshots
Not applicable.
Bug fix verification
apps/daemon/tests/artifacts/standalone-html.test.ts and e2e/ui/app-manual-edit.test.tsmain: yesValidation
mise exec -- pnpm guardmise exec -- pnpm typecheckmise exec -- pnpm --filter @open-design/contracts test (289 passed)focused daemon Vitest suite for standalone export and routing (66 passed)focused web Vitest suite for export runtime and FileViewer (371 passed)focused Playwright file:// offline export workflow (1 passed)