diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index de5fac93..68e35f69 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -118,6 +118,12 @@ jobs: - name: check frontend run: npm run check + # The code conventions no type can express (one implementation of a fixed + # behaviour, Rust reached only through the generated bindings, ...), as + # lint rules in eslint.config.js. Each message says why. + - name: lint frontend conventions + run: npm run lint + - name: run frontend behavior tests run: npm test @@ -127,6 +133,17 @@ jobs: - name: run frontend behavior tests (vitest) run: npm run test:vitest + # Monaco is most of the app's JavaScript and only the editor needs it, so + # Editor.svelte loads it with `await import()`. A static import anywhere in + # the startup graph folds it back into the chunks every window parses on + # first paint, and nothing else fails. This builds the frontend and reads + # the Vite manifest (which chunks load at startup) and their source maps + # (which modules each chunk holds). Measured: about 6 seconds. + - name: monaco is not in the startup chunks + run: | + npx vite build --sourcemap + node scripts/check-startup-chunk.mjs + # Nothing had ever checked Rust formatting, so master drifted to 47 # rustfmt diffs across all six source files -- not because anyone # disagreed with the tool, but because `cargo fmt` is a thing you have to @@ -143,10 +160,19 @@ jobs: cd src-tauri cargo fmt --check + # `--locked` fails the run when Cargo.lock no longer matches Cargo.toml + # (a version bump that skipped the lock), instead of rewriting it here. - name: run cargo test run: | cd src-tauri - cargo test + cargo test --locked + + # `cargo test` just ran `export_bindings`, which rewrites + # src/lib/bindings.ts from the registered commands and events. A Rust + # command or event changed without the regenerated file committed leaves a + # diff here, before the frontend can drift from what Rust answers. + - name: check generated bindings are committed + run: git diff --exit-code src/lib/bindings.ts # `-D warnings` is only defensible because the count is currently zero: # the two that existed (a redundant `'static` on a const, a `push_str` @@ -159,4 +185,4 @@ jobs: - name: run clippy run: | cd src-tauri - cargo clippy --all-targets -- -D warnings + cargo clippy --locked --all-targets -- -D warnings diff --git a/AGENTS.md b/AGENTS.md index 0266ba44..26f511c1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -143,10 +143,11 @@ src-tauri/ - Export singleton instances (e.g., `export const tabManager = new TabManager()`) ### Tauri Invoke -Use `invoke()` from `@tauri-apps/api/core` for Rust commands: +Call Rust commands through the generated bindings (`cargo test` regenerates +`src/lib/bindings.ts`); a raw `invoke` is a lint error: ```typescript -import { invoke } from '@tauri-apps/api/core'; -const result = await invoke('command_name', { arg: value }); +import { commands } from '../bindings.js'; // relative to the caller +const result = await commands.commandName(value); ``` ### File Operations @@ -158,8 +159,8 @@ All file operations go through Rust commands - never use Node.js fs APIs. - **Frontend**: two runners, split by glob so a file belongs to exactly one. - `npm test` runs `scripts/*.test.ts` (`node --test --import tsx`). - `npm run test:vitest` runs `scripts/*.spec.ts` (vitest, jsdom, Svelte plugin). -- **CI**: Runs `npm audit`, `npm run check`, `npm test`, `npm run test:vitest` and - `cargo test` on PRs +- **CI**: Runs `npm audit`, `npm run check`, `npm run lint`, `npm test`, + `npm run test:vitest` and `cargo test` on PRs Tests run the real code and assert what it does. A test does not read source as text — no regex over a file, no slicing a function body, no cutting code out of a component and @@ -167,10 +168,12 @@ evaluating it: such a test guards a spelling, goes red on a rename and stays gre regression. When the logic sits in a component where a test cannot call it, move it into a `.ts` or `.svelte.ts` module the component calls, and test the module. -The `*.checks.*` files are the one exception, and are meant to shrink: they hold contract -checks (a Tauri command or event name, a config key, an i18n key — the spelling that joins -Rust and TypeScript) and convention checks ("one implementation of this behavior"), until -those move to generated bindings and lint rules. +What a run cannot show is held elsewhere. Command and event names, arguments and returns +are generated from Rust into `src/lib/bindings.ts` (tauri-specta), so a mismatch is a +`npm run check` error. Code conventions ("one implementation of this behavior", "call +through this wrapper") are lint rules in `eslint.config.js` (`npm run lint`), plus +`src-tauri/clippy.toml` and `scripts/conventions.test.ts` for the Rust ones clippy cannot +express. ### Where a test goes @@ -178,8 +181,6 @@ Tests are grouped by feature, not by the fix that added them: - `scripts/.test.ts` and `scripts/.spec.ts` hold behavior tests (the runner rules below decide which of the two). -- `scripts/.checks.test.ts` / `.checks.spec.ts` hold the cases that read source as - text, kept apart from the behavior tests. - `scripts/..test.ts` / `.spec.ts` is for a part of a feature that must run in its own process (module mocks, a process-wide shim such as `installShimDom()`) or would make the main file unreadable. @@ -279,7 +280,7 @@ mandatory for `tauri-plugin-updater` to compare versions correctly. The lock file is the one that gets forgotten, because nothing in the editing loop reads it — a bump without it lands green and stays wrong until the next person's `cargo build` rewrites the line and leaves them a dirty working tree. -`scripts/release.checks.test.ts` asserts the three agree. +`scripts/release.workflows.test.ts` asserts the three agree. ## Releasing diff --git a/RELEASING.md b/RELEASING.md index 8f78a3fa..8817b5c0 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -44,7 +44,7 @@ If you ever lose the private key: ### 5. `alecdotdev/Markpad` is load-bearing -The updater endpoint in `src-tauri/tauri.conf.json` points at **`sftwrdotdev/Markpad`**. (`scripts/release.checks.test.ts` holds that name and the endpoint together — if the repository ever moves again, this line moves with it.) +The updater endpoint in `src-tauri/tauri.conf.json` points at **`sftwrdotdev/Markpad`**. (`scripts/release.workflows.test.ts` holds that name and the endpoint together — if the repository ever moves again, this line moves with it.) That only helps builds made from here on. The endpoint is compiled into the binary, so every copy up to and including v2.7.0 asks GitHub for `https://github.com/alecdotdev/Markpad/releases/latest/download/latest.json` and reaches the current feed only because GitHub answers with a 301. Those installs use that redirect for as long as they run. @@ -112,7 +112,7 @@ Back it up in the same place as the minisign private key, and treat it with the ## Per-release workflow -The workflow uses `npm ci`, so its installed dependency graph is exactly the committed lockfile. Do not replace it with `npm install` in release jobs. `scripts/release.checks.test.ts` guards that. +The workflow uses `npm ci`, so its installed dependency graph is exactly the committed lockfile. Do not replace it with `npm install` in release jobs. `scripts/release.workflows.test.ts` guards that. [`snapcraft.yaml`](snapcraft.yaml) builds nothing. It packages the `.deb` the release already shipped (`plugin: dump`, `source-type: deb`), so the snap carries the same binary as the `.deb` and the AppImage. The same test asserts it is packaging rather than building. diff --git a/eslint.config.js b/eslint.config.js new file mode 100644 index 00000000..d49eaccb --- /dev/null +++ b/eslint.config.js @@ -0,0 +1,656 @@ +// Markpad's code conventions, as lint rules (`npm run lint`). +// +// This config turns on no style preset. Every rule below is a convention the +// app depends on that no type can express: a second copy of a fixed behaviour, +// a call that must go through one wrapper, a defect shape that must not come +// back. Each message says why. +// +// The rules are `no-restricted-syntax` selectors, but not the built-in rule. In +// a flat config a later block REPLACES an earlier block's options for the same +// rule, so two conventions covering MarkdownViewer.svelte would silently drop +// one of them. `convention()` builds one rule per convention instead, so each +// keeps its own files, its own exceptions and its own name in the report. +// Selectors run on the TypeScript AST and, in `.svelte` files, on the template +// AST of svelte-eslint-parser (`SvelteMustacheTag`, `SvelteLiteral`, ...), so +// comments and prose never match. +import svelte from 'eslint-plugin-svelte'; +import tseslint from 'typescript-eslint'; + +/** + * @typedef {object} Convention + * @property {string[]} [files] globs, default every source file under src/ + * @property {string[]} [ignores] files the convention allows + * @property {Record} [ban] selector -> message, reported on every match + * @property {Record} [atMost] selector -> [n, message], reported past the nth match in a file + * @property {{ call: string, earlier: string, message: string }} [requireEarlier] + * every call of `call` needs a call of `earlier` before it in the same function + * (callbacks passed as arguments count as part of the function around them) + * @property {import('eslint').Rule.RuleModule} [custom] a rule of its own, for what a selector cannot say + */ + +/** + * @param {Convention} c + * @returns {import('eslint').Rule.RuleModule} + */ +function convention(c) { + return { + meta: { type: 'problem', schema: [] }, + create(context) { + /** @type {Record void>} */ + const visitors = {}; + for (const [selector, message] of Object.entries(c.ban ?? {})) { + visitors[selector] = (node) => context.report({ node, message }); + } + for (const [selector, [n, message]] of Object.entries(c.atMost ?? {})) { + let seen = 0; + visitors[selector] = (node) => { + if (++seen > n) context.report({ node, message }); + }; + } + if (c.requireEarlier) { + const { call, earlier, message } = c.requireEarlier; + /** @type {(node: any, name: string) => boolean} */ + const named = (node, name) => node.callee.name === name || node.callee.property?.name === name; + /** @type {(node: any) => boolean} */ + const isFunction = (node) => /Function/.test(node.type) && node.parent.type !== 'CallExpression'; + /** @type {number[]} */ + const earlierStarts = []; + visitors.CallExpression = (/** @type {any} */ node) => { + if (named(node, earlier)) earlierStarts.push(node.range[0]); + if (!named(node, call)) return; + const fn = context.sourceCode.getAncestors(node).findLast(isFunction); + const from = fn?.range?.[0] ?? 0; + if (!earlierStarts.some((start) => start >= from && start < node.range[0])) { + context.report({ node, message }); + } + }; + } + return visitors; + }, + }; +} + +/** `aria-valuemin={X_RANGE.min}` next to `aria-valuemax={X_RANGE.max}`, the same X_RANGE. */ +/** @type {import('eslint').Rule.RuleModule} */ +const ariaRange = { + meta: { type: 'problem', schema: [] }, + create(context) { + /** @type {(attr: any, end: string) => string | null} */ + const range = (attr, end) => { + const [value] = attr?.value ?? []; + const e = attr?.value.length === 1 && value.type === 'SvelteMustacheTag' ? value.expression : null; + return e?.type === 'MemberExpression' && e.property.name === end && /_RANGE$/.test(e.object.name ?? '') + ? e.object.name + : null; + }; + return { + /** @param {any} node */ + SvelteStartTag(node) { + /** @type {(name: string) => any} */ + const attr = (name) => node.attributes.find((/** @type {any} */ a) => a.type === 'SvelteAttribute' && a.key.name === name); + const min = attr('aria-valuemin'); + const max = attr('aria-valuemax'); + if (!min && !max) return; + const name = range(min, 'min'); + if (!name || range(max, 'max') !== name) { + context.report({ + node, + message: + "aria-valuemin/max is the bound assistive tech reports; advertise the setting's own *_RANGE.min/.max " + + '(the one the store clamps against), not numbers of its own, so the two cannot drift.', + }); + } + }, + }; + }, +}; + +const VIEWER = 'src/lib/MarkdownViewer.svelte'; +const EDITOR = 'src/lib/components/Editor.svelte'; +const BINDINGS = 'src/lib/bindings.ts'; +const SANITIZER_FILES = ['src/lib/utils/richContent.ts', 'src/lib/utils/sanitize.ts']; +const callOf = (/** @type {string} */ name) => `CallExpression[callee.name='${name}'], CallExpression[callee.property.name='${name}']`; +const command = (/** @type {string} */ name) => `CallExpression[callee.object.name='commands'][callee.property.name='${name}']`; + +/** @type {Record} */ +const CONVENTIONS = { + // ------------------------------------------------------------ Tauri boundary + 'tauri-through-bindings': { + ignores: [BINDINGS, 'src/lib/windowEvents.ts'], + ban: { + "ImportDeclaration[source.value=/^@tauri-apps\\/api\\/(core|event)$/] > ImportSpecifier[imported.name=/^(invoke|listen|once|emit|emitTo)$/]": + 'Call Rust through the generated `commands.*` and listen to Rust events through `events.*` (src/lib/bindings.ts): ' + + 'a raw invoke/listen names the command or event by a string tsc cannot check. Events between windows go through src/lib/windowEvents.ts.', + }, + }, + + // ------------------------------------------------------------------ documents + 'live-mode-toggle-does-not-reload': { + files: [VIEWER], + ban: { + "FunctionDeclaration[id.name='toggleLiveMode'] CallExpression[callee.name='loadMarkdown']": + 'Turning Live Mode on is not a request to discard the buffer; reloading here skips canCloseTab.', + }, + }, + 'view-modes-do-not-touch-live-mode': { + files: ['src/lib/sessions/viewMode.svelte.ts'], + ban: { + "Identifier[name=/^(liveMode|toggleLiveMode)$/]": + 'Entering split view used to turn Live Mode off behind the user (#692). The view modes are not handed Live Mode.', + }, + }, + 'open-dialog-uses-the-extension-list': { + ignores: [BINDINGS], + ban: { + "Property[key.name='extensions'] > ArrayExpression:has(> Literal[value='md'])": + 'A literal extension list drifts from the one Markpad treats as documents; use MARKDOWN_OPEN_FILTERS (utils/markdownLinks.ts).', + }, + }, + 'save-content-cancels-auto-save-itself': { + ban: { + [`ExpressionStatement:has(> :matches(${callOf('cancelPendingAutoSave')})) + :matches(ExpressionStatement, VariableDeclaration, ReturnStatement):has(${callOf('saveContent')})`]: + 'saveContent already cancels the pending auto-save; pairing a manual cancel with it moves the duty back to the call sites, where the next one forgets it.', + }, + }, + 'only-the-revert-commands-discard-a-buffer': { + ignores: ['src/lib/sessions/externalChange.svelte.ts'], + ban: { + "Property[key.name='discardUnsavedBuffer'][value.value=true]": + 'Only the two revert commands (Reload on the conflict bar, Reload from disk) may say the user chose to lose work.', + }, + }, + + // -------------------------------------------------------------------- editing + 'one-copy-reference-builder': { + ignores: ['src/lib/utils/headingReference.ts'], + ban: { + "CallExpression[callee.name='referenceForTab']:not(FunctionDeclaration[id.name='copyHeadingReference'] *)": + 'Copy Reference has one builder (copyHeadingReference in MarkdownViewer.svelte); the three menu entries used to build the string inline and disagreed.', + }, + }, + + // --------------------------------------------------------------------- layout + 'outline-takes-no-document-string': { + files: ['src/lib/components/Toc.svelte'], + ban: { + "Identifier[name='htmlContent']": + 'The outline follows the revision the preview patch publishes, not the rendered document string.', + }, + }, + 'split-ratio-is-clamped-by-the-store': { + files: ['src/lib/utils/splitPanes.ts'], + ban: { + "MemberExpression[object.name='Math'][property.name=/^(min|max)$/]": + 'TabManager.setSplitRatio clamps; a second copy of the bounds here drifts from it.', + }, + }, + 'splitter-direction-has-one-source': { + files: [VIEWER], + ban: { + "ConditionalExpression[test.right.value='left']:matches([test.left.name='splitEditorSide'], [test.left.property.name='splitEditorSide'])": + 'Both splitter routes take the direction from the same place; a third route spelling it is the drift.', + }, + }, + 'fold-layout-is-rooted-at-the-visible-host': { + files: [VIEWER], + ban: { + "CallExpression[callee.name='observeFoldLayout'][arguments.0.name='markdownBody'], CallExpression[callee.name='followFoldLayout'][arguments.1.body.name='markdownBody']": + 'The article holds one host per tab, all but one display:none; measuring from it writes zero heights into every hidden tab.', + }, + }, + + // ----------------------------------------------------------------- navigation + 'one-preview-restore-cascade': { + files: [VIEWER], + ban: { + "AssignmentExpression[left.property.name='scrollTop'][right.callee.name='getAnchorScrollTop'], MemberExpression[property.name='scrollPercentage'], CallExpression[callee.object.name='Array'][callee.property.name='from'][arguments.0.property.name='children']": + 'The reading position is restored through restorePreviewReadingPosition (utils/previewAnchor.ts); the cascade is an ORDER, and a second site consulting the same fields answers differently.', + }, + }, + 'line-coordinates-come-from-the-buffer': { + files: [VIEWER], + ban: { + "CallExpression[callee.name='lineCoordinates']:not(VariableDeclarator[id.name='lineCoords'] > CallExpression > CallExpression[arguments.0.name='rawContent'])": + 'One derivation, `lineCoords = $derived(lineCoordinates(rawContent))`: the render is what the front matter was stripped out of, so measuring it answers 0.', + }, + }, + 'editor-derives-the-anchor-once': { + files: [EDITOR, 'src/lib/utils/editorDocument.svelte.ts'], + ban: { + "Identifier[name='tabAnchorForEditorTopLine']": + 'The editor crosses from buffer to renderer numbering in editorReadingPosition (utils/editorPosition.ts) only.', + }, + atMost: { + [callOf('editorReadingPosition')]: [1, 'The editor derives its reading position in one place.'], + }, + }, + + // --------------------------------------------------------------------- editor + 'one-update-options-effect': { + files: [EDITOR], + atMost: { + "CallExpression[callee.object.name='editor'][callee.property.name='updateOptions']": [ + 1, + 'Two effects writing the same Monaco options (fontSize with and without zoom) made the winner depend on effect order.', + ], + }, + }, + 'settings-os-type-is-written-by-the-store': { + ignores: ['src/lib/stores/settings.svelte.ts'], + ban: { + "AssignmentExpression[left.object.name='settings'][left.property.name='osType']": + 'settings.osType is filled in by the store from get_os_type; ask platformOf() instead of writing it.', + }, + }, + 'editor-clipboard-goes-through-editor-clipboard': { + files: [EDITOR], + ban: { + [command('clipboardWriteText')]: + 'Cut and copy write the clipboard in utils/editorClipboard.ts, so they cannot drift apart and the whole-line rule is implemented once.', + }, + }, + 'one-cut-one-copy': { + files: ['src/lib/utils/editorClipboard.ts'], + atMost: { + [command('clipboardWriteText')]: [ + 2, + 'copyToClipboard and cutToClipboard are the editor\'s two clipboard writers; a third derives the text again.', + ], + }, + }, + 'monaco-theme-writers': { + ignores: [EDITOR, 'src/lib/utils/mountEditor.ts', 'src/lib/utils/theme.ts'], + ban: { + "CallExpression[callee.object.property.name='editor'][callee.property.name='setTheme']": + 'The editor pane, its mount and the theme importer (which defines `vscode-custom`) are the only Monaco theme writers.', + }, + }, + 'an-edit-is-its-own-undo-step': { + files: [EDITOR, 'src/lib/utils/editorClipboard.ts'], + requireEarlier: { + call: 'executeEdits', + earlier: 'pushUndoStop', + message: + 'executeEdits appends to the undo element typing left open, so Cmd+Z takes the typing too; call editor.pushUndoStop() before it.', + }, + }, + 'on-mount-is-synchronous': { + files: [EDITOR], + ban: { + "CallExpression[callee.name='onMount'] > :matches(ArrowFunctionExpression, FunctionExpression)[async=true]": + 'Svelte only takes a synchronously returned function as the unmount cleanup; an async onMount leaks what it set up.', + }, + }, + 'editor-effects-wait-for-editor-ready': { + files: [EDITOR], + ban: { + "CallExpression[callee.name='$effect']:has(Identifier[name='editor']):not(:has(Identifier[name='editorReady']))": + '`editor` is a plain let assigned after an await; an effect gated on it alone runs once against nothing and never again.', + }, + }, + 'monaco-is-loaded-on-demand': { + ban: { + "ImportDeclaration[importKind!='type'][source.value=/^monaco-(editor|vim)($|\\/)/]:not([source.value=/[?]worker$/]), ExportNamedDeclaration[exportKind!='type'][source.value=/^monaco-(editor|vim)($|\\/)/], ExportAllDeclaration[exportKind!='type'][source.value=/^monaco-(editor|vim)($|\\/)/]": + 'Monaco is most of the startup JavaScript and only the editor needs it: load it with `await import()` (Editor.svelte), or `import type`.', + }, + }, + 'one-image-link-writer': { + files: [EDITOR, 'src/lib/utils/editorClipboard.ts'], + ban: { + "Literal[value=/%20/], TemplateElement[value.raw=/%20/]": + 'A hand-rolled space escape is back; imageEmbed (utils/imageEmbed.ts) escapes the destination.', + }, + }, + 'one-image-directory-resolver': { + ignores: ['src/lib/utils/imageEmbed.ts'], + ban: { + "LogicalExpression[operator='||'][left.object.name='settings'][left.property.name='imageDirectory'], LogicalExpression[operator='||'][right.name='DEFAULT_IMAGE_DIRECTORY'], LogicalExpression[operator='||'][right.value='img']": + 'Paste, drop and the path completion read the image directory through one resolver (utils/imageEmbed.ts), so `${filename}` expands at all three.', + }, + }, + 'delete-line-stays-bound': { + files: [EDITOR], + ban: { + "Literal[value='-editor.action.deleteLines']": + 'Mod+Shift+K deletes a line, as in Monaco (#821); unbinding it took a key users asked for back.', + }, + }, + + // -------------------------------------------------------------------- preview + 'katex-contribs-through-the-exports-map': { + ban: { + "ImportExpression[source.value=/^katex\\/dist\\/contrib\\//], ImportDeclaration[source.value=/^katex\\/dist\\/contrib\\//]": + 'katex/dist/contrib/*.js is CommonJS with its own KaTeX copy, so a macro mhchem registers there never reaches renderToString (#745). Import katex/contrib/*.', + }, + }, + 'one-copy-handler': { + ban: { + "ImportExpression[source.value=/copy-tex/], ImportDeclaration[source.value=/copy-tex/]": + 'copy-tex rewrote the clipboard for selections with math and undid the preview copy handler (#674); its behaviour is utils/previewCopy.ts.', + "CallExpression[callee.property.name='addEventListener'][arguments.0.value='copy']": + 'The preview copy handler writes the clipboard itself (#549, #674); a second copy listener lets the webview decide again.', + }, + }, + 'viewer-markup-sinks': { + files: [VIEWER], + ban: { + "SvelteMustacheTag[kind='raw']:not([expression.object.name='tooltip'][expression.property.name='html'])": + 'The document reaches the article through patchHost (the sanitizer\'s output); the footnote tooltip re-injects a clone of that DOM. Any other {@html} is a new unsanitized sink.', + "Identifier[name='ALLOWED_URI_REGEXP']": + 'The viewer must not carry its own copy of the URI policy (utils/sanitize.ts).', + "CallExpression[callee.name='processMarkdownHtml']": + 'The preview processes the blocks it inserts in processAndSanitize (utils/previewPatch.ts) and nowhere else.', + }, + atMost: { + "CallExpression[callee.name='patchHost']": [1, 'The preview reaches the article through one patchHost call.'], + }, + }, + 'one-sanitizer-policy': { + ignores: SANITIZER_FILES, + ban: { + "ImportDeclaration[source.value='dompurify'], CallExpression[callee.object.name='DOMPurify'][callee.property.name='sanitize']": + 'A DOMPurify config outside utils/sanitize.ts (documents) and utils/richContent.ts (Mermaid SVG) is a second, unreviewed sanitizer. Use sanitizeMarkdownHtml.', + }, + }, + 'diagram-sanitizer-stays-separate': { + files: ['src/lib/utils/richContent.ts'], + ban: { + "ImportDeclaration[source.value=/\\/sanitize(\\.js)?$/]": + 'The diagram filter needs the ${body}`, media); +} + +function exportPage(media: Media, articleHtml: string, contentWidth: number | null = DEFAULT_PREVIEW_MAX_WIDTH): Window { + const html = buildExportDocument({ theme: 'light', title: 'T', styles: appStyles, articleHtml, contentWidth, appearance: plainAppearance }); + return page(html, media); +} + +/** The computed `property` of the element `selector` picks in `window`. */ +function computed(window: Window, selector: string, property: string): string { + const element = window.document.querySelector(selector); + assert.ok(element, `fixture has no ${selector}`); + return window.getComputedStyle(element).getPropertyValue(property); +} + +describe('printing an exported HTML file', () => { + const pdf = () => appPage('
', 'print'); + + test('prints the article instead of a blank page', () => { + // The copied print sheet hides everything outside #app, which an exported + // file does not have, so the article itself was hidden on paper. + assert.notEqual(computed(exportPage('print', '

body

'), '.markdown-body', 'display'), 'none'); + }); + + test('gets the same paper margins as the app’s own PDF', () => { + // The export used to keep its 40px screen padding on paper. + const paper = computed(pdf(), '.markdown-body', 'padding-top'); + assert.ok(parseFloat(paper) > 0, `the PDF has no page margin: ${paper}`); + const printed = exportPage('print', '

body

'); + assert.equal(computed(printed, '.markdown-body', 'padding-top'), paper); + assert.equal(computed(printed, '.markdown-body', 'padding-left'), paper); + }); + + test('looks the same on screen as on paper', () => { + // One page margin for both, so the file does not change shape when printed. + const screen = computed(exportPage('screen', '

body

'), '.markdown-body', 'padding-top'); + assert.ok(parseFloat(screen) > 0, `no margin on screen: ${screen}`); + assert.equal(screen, computed(exportPage('print', '

body

'), '.markdown-body', 'padding-top')); + assert.equal( + computed(exportPage('screen', '

body

'), '.markdown-body', 'padding-left'), + computed(exportPage('print', '

body

'), '.markdown-body', 'padding-left'), + ); + }); +}); + +describe('exported content width', () => { + test('the export cap loses to the print block, so the PDF route is unchanged', () => { + // `@media print` in styles.css forces `.markdown-body` to `max-width: 100%` + // with its own page-margin padding, and that block travels into the export with + // the rest of the sheet. Printing an exported file never saw the width + // setting and must not start seeing it. + const printed = exportPage('print', '

body

', MIN_PREVIEW_MAX_WIDTH); + assert.equal(computed(printed, '.markdown-body', 'max-width'), '100%'); + // The PDF route itself prints the live viewer's `#print-root`, not an export. + // 1cm, computed to px the way jsdom computes it. + const viewer = appPage('
', 'print'); + const oneCm = computed(page('
', 'screen'), '.markdown-body', 'padding-top'); + assert.equal(computed(viewer, '.markdown-body', 'max-width'), '100%'); + assert.equal(computed(viewer, '.markdown-body', 'padding-top'), oneCm); + assert.equal(computed(viewer, '.markdown-body', 'padding-left'), oneCm); + }); + + test('the app stylesheet copied into an export does not out-rank the cap', () => { + // The export appends its rules after the copied sheet, so with real styles in + // front of them the screen cap still has to be the configured one. + assert.equal(computed(exportPage('screen', '

body

', 1240), '.markdown-body', 'max-width'), '1240px'); + assert.equal(computed(exportPage('screen', '

body

', null), '.markdown-body', 'max-width'), 'none'); + }); +}); + +describe('folds in print and export', () => { + // The HTML route renders every heading fold open in TypeScript. The print + // route prints the live DOM with its folds still collapsed, so on paper a + // collapsed section surviving is a cascade question. + const article = `
+ + +

open

+ +

open

+
`; + + const paper = appPage(`
`, 'print'); + const screen = appPage(`
${article}
`, 'screen'); + const exported = exportPage('screen', article); + + test('a collapsed heading section is printed, not clipped away', () => { + // The PDF is the copy the reader gets. Clipping a fold to zero height + // deletes text from it with no sign that anything is missing. + assert.equal(computed(paper, '#collapsed-section', 'height'), 'auto'); + assert.notEqual(computed(paper, '#collapsed-section', 'overflow-y'), 'hidden'); + // Height alone is not enough: the screen rule pairs it with opacity: 0. + assert.notEqual(computed(paper, '#collapsed-section', 'opacity'), '0'); + assert.notEqual(computed(paper, '#collapsed-section-inner', 'overflow-y'), 'hidden'); + }); + + test('a collapsed callout is printed too', () => { + // Callouts collapse with a 0fr grid track instead of a height, so a fix + // aimed only at headings leaves half the document disappearing. + assert.equal(computed(paper, '#collapsed-callout-content', 'grid-template-rows'), 'minmax(0, 1fr)'); + assert.notEqual(computed(paper, '#collapsed-callout-content', 'opacity'), '0'); + assert.notEqual(computed(paper, '#collapsed-callout-inner', 'overflow-y'), 'hidden'); + }); + + test('folding still works on screen', () => { + // The print rules reveal the fold. They must not disable the feature. + assert.equal(computed(screen, '#collapsed-section', 'height'), '0px'); + assert.equal(computed(screen, '#collapsed-section', 'opacity'), '0'); + assert.equal(computed(screen, '#collapsed-callout-content', 'grid-template-rows'), 'minmax(0, 0fr)'); + }); + + test('an open callout sizes its row with minmax, not a bare fr', () => { + // #850. WebKit keeps the height a bare `1fr` row was given at a narrower + // width: a callout laid out in the zero-width preview pane stayed ~500px tall + // after Ctrl+E widened the pane to 860px. Measured in a WKWebView. + // `minmax(0, 1fr)` re-sizes it. jsdom has no layout, so the value is what + // this can hold. + assert.equal(computed(screen, '#open-callout-content', 'grid-template-rows'), 'minmax(0, 1fr)'); + assert.equal(computed(paper, '#open-callout-content', 'grid-template-rows'), 'minmax(0, 1fr)'); + }); + + test('an expanded section is unaffected by the reveal', () => { + assert.notEqual(computed(paper, '#open-section', 'height'), '0px'); + assert.notEqual(computed(paper, '#open-section', 'opacity'), '0'); + }); + + test('the fold controls do not print as misleading decoration', () => { + // A collapsed chevron points at content that now prints expanded, and it + // is a control for an interaction paper does not have. + assert.equal(computed(paper, '#collapsed-heading .header-fold-icon', 'display'), 'none'); + assert.equal(computed(paper, '#callout-chevron', 'display'), 'none'); + }); + + test('folding a section turns its own chevron, not the ones nested inside it', () => { + // A heading's content wrapper holds the headings below it, and a callout's + // content can hold another callout. `.is-collapsed .header-fold-icon` matched + // every one of those, so they turned while the section animated shut. + assert.equal(computed(screen, '#collapsed-heading .header-fold-icon', 'transform'), 'rotate(-90deg)'); + assert.equal(computed(screen, '#nested-heading .header-fold-icon', 'transform'), 'none'); + assert.equal(computed(screen, '#nested-heading .header-fold-icon', 'opacity'), '0'); + assert.equal(computed(screen, '#callout-chevron', 'transform'), 'rotate(-90deg)'); + assert.equal(computed(screen, '#nested-callout-chevron', 'transform'), 'none'); + }); + + test('a callout collapsed in the source is still readable in an exported file', () => { + // `> [!note]-` puts `is-collapsed` in the markup itself, and an exported file + // has no toggle and a policy that forbids the script one. Verified in Chrome + // against a real exported file before the fix: the collapsed callout's + // content box measured 0px at opacity 0. + assert.equal(computed(exported, '#collapsed-callout-content', 'grid-template-rows'), 'minmax(0, 1fr)'); + assert.notEqual(computed(exported, '#collapsed-callout-content', 'opacity'), '0'); + assert.notEqual(computed(exported, '#collapsed-callout-inner', 'overflow-y'), 'hidden'); + // The same for a heading fold, whatever fold state a caller hands the renderer. + assert.notEqual(computed(exported, '#collapsed-section', 'height'), '0px'); + assert.notEqual(computed(exported, '#collapsed-section', 'opacity'), '0'); + }); + + test('an exported file does not offer controls it cannot honour', () => { + assert.equal(computed(exported, '#callout-chevron', 'display'), 'none'); + }); +}); + +describe('oversized media in print', () => { + // `@page { margin: 0 }` makes the page box the whole sheet, and `.markdown-body` + // pads 1cm top and bottom, so the printable column is the sheet minus 2cm. + const MARGINS_IN = 2 / 2.54; + const LETTER_PRINTABLE_IN = 11 - MARGINS_IN; + const A4_PRINTABLE_IN = 11.69 - MARGINS_IN; + + const body = `
+ `; + const paper = appPage(body, 'print'); + const screen = appPage(body, 'screen'); + + /** jsdom computes absolute lengths to px (96 per inch). A relative cap stays relative and fails here. */ + function inches(value: string): number { + const match = value.match(/^([\d.]+)px$/); + assert.ok(match, `the cap must be an absolute length, not: ${value}`); + return Number(match[1]) / 96; + } + + test('the print block caps media height at a size both papers can hold', () => { + // A replaced element cannot be fragmented, so `break-inside: avoid` alone + // turns anything taller than the column into a cropped image. + // jsdom resolves vh/% against its window, so the cap is computed at two + // window heights: only an absolute length reads the same at both. + const shortWindow = appPage(body, 'print'); + Object.defineProperty(shortWindow, 'innerHeight', { value: 100 }); + for (const selector of ['#image', '#diagram-svg']) { + assert.equal(computed(shortWindow, selector, 'max-height'), computed(paper, selector, 'max-height'), 'the cap must not depend on the window'); + const cap = inches(computed(paper, selector, 'max-height')); + assert.ok(cap <= LETTER_PRINTABLE_IN, `${cap}in overflows Letter (${LETTER_PRINTABLE_IN}in)`); + assert.ok(cap <= A4_PRINTABLE_IN, `${cap}in overflows A4 (${A4_PRINTABLE_IN}in)`); + assert.ok(cap >= 6, `${cap}in shrinks every full-page figure needlessly`); + } + }); + + test('oversized images are scaled to a page instead of being clipped', () => { + assert.notEqual(computed(paper, '#image', 'max-height'), computed(screen, '#image', 'max-height')); + inches(computed(paper, '#image', 'max-height')); + }); + + test('oversized mermaid diagrams are scaled to a page too', () => { + inches(computed(paper, '#diagram-svg', 'max-height')); + }); + + test('the cap preserves aspect ratio rather than squashing', () => { + // `height: auto` is what lets the cap scale the width down proportionally. + assert.equal(computed(paper, '#image', 'object-fit'), 'contain'); + assert.equal(computed(paper, '#image', 'height'), 'auto'); + assert.equal(computed(paper, '#image', 'max-width'), '100%'); + }); + + test('the diagram container does not clip the capped diagram', () => { + // The viewer gives `.mermaid-diagram` `overflow-x: auto` for the screen, which + // computes overflow-y to `auto` as well and would crop on paper. jsdom does + // not apply that coupling, so overflow-x is the value this can hold. + assert.equal(computed(screen, '#diagram', 'overflow-x'), 'auto'); + assert.equal(computed(paper, '#diagram', 'overflow-x'), 'visible'); + }); + + test('the media caps do not undo the existing print handling', () => { + // #359 moved diagram colours out of CSS into a light-theme re-render + // (utils/mermaidPrint.ts). Paper must paint the diagram as the screen does. + // The fixture's `
`; - -describe('printing an exported HTML file', () => { - test('prints the article instead of a blank page', () => { - // The copied print sheet hides everything outside #app, which an exported - // file does not have, so the article itself was hidden on paper. - assert.notEqual(article(exported, 'print').display, 'none'); - }); - - test('gets the same paper margins as the app’s own PDF', () => { - // The export used to keep its 40px screen padding on paper. - const pdf = article(pdfPage, 'print'); - assert.ok(parseFloat(pdf.paddingTop) > 0, `the PDF has no page margin: ${pdf.paddingTop}`); - const printed = article(exported, 'print'); - assert.equal(printed.paddingTop, pdf.paddingTop); - assert.equal(printed.paddingLeft, pdf.paddingLeft); - }); - - test('looks the same on screen as on paper', () => { - // One page margin for both, so the file does not change shape when printed. - const screen = article(exported, 'screen'); - const paper = article(exported, 'print'); - assert.ok(parseFloat(screen.paddingTop) > 0, `no margin on screen: ${screen.paddingTop}`); - assert.equal(screen.paddingTop, paper.paddingTop); - assert.equal(screen.paddingLeft, paper.paddingLeft); - }); -}); diff --git a/scripts/export.test.ts b/scripts/export.test.ts index f92f7145..fe574c5b 100644 --- a/scripts/export.test.ts +++ b/scripts/export.test.ts @@ -15,7 +15,21 @@ import { resetRichContentCache } from '../src/lib/utils/richContentCache.ts'; import { ALLOWED_MARKDOWN_URI_REGEXP } from '../src/lib/utils/sanitize.js'; import { MERMAID_PRINT_THEME, readDiagramSource, rememberDiagramSource, resolveMermaidTheme } from '../src/lib/utils/mermaidPrint.js'; import { useShimDom } from './preview.domShim.ts'; +import { readSource } from './sourceTree.js'; import DOMPurify from 'dompurify'; +import postcss, { type Rule } from 'postcss'; + +/** The app stylesheet, parsed once: the CSS cases below assert on its rules. */ +const appStylesheet = postcss.parse(readSource('src/styles.css')); + +/** Every selector in the sheet, one per comma-separated entry, with the rule it belongs to. */ +function appSelectors(): { selector: string; rule: Rule }[] { + const found: { selector: string; rule: Rule }[] = []; + appStylesheet.walkRules((rule) => { + for (const selector of rule.selectors) found.push({ selector, rule }); + }); + return found; +} /** * #467: "Is there a way how to change the width of exported HTML?" — there was @@ -34,8 +48,8 @@ import DOMPurify from 'dompurify'; */ describe('exported content width', () => { // The shim stands in for the DOM `exportAsHtml` builds into. Without a real DOM - // DOMPurify is a no-op object; what it does and where it sits in the pipeline is - // `export.checks.test.ts`'s subject, not this one's. + // DOMPurify is a no-op object; what it does is preview.spec.ts's subject, not + // this one's. useShimDom({ window: globalThis, location: { href: 'http://tauri.localhost/' } }, { identitySanitize: true }); type Rule = { selectors: string[]; declarations: string; media: 'screen' | 'print' }; @@ -647,8 +661,8 @@ describe('export carries the preview appearance', () => { * — which elements get handed to which library, and where the results end * up — and not the libraries' own output. * - `DOMPurify.sanitize`, which is a no-op object without a real DOM. It is - * replaced by the identity function. What the filter does and where it sits - * in the pipeline is pinned by `export.checks.test.ts`; the relevant fact + * replaced by the identity function. What the filter does runs in + * `preview.spec.ts`; the relevant fact * here is only that the diagram filter and the document filter are separate * calls, which the diagram test below covers from the other side. */ @@ -1060,6 +1074,39 @@ describe('rich content in the HTML export', () => { root.innerHTML = '

plain text

'; assert.equal(collectUsedKatexFamilies(root, KATEX_CSS).size, 0); }); + + test('the installed KaTeX still gives the exporter fonts it can find', async () => { + // `KATEX_CSS` above is a slice of katex.min.css frozen at the version it was + // written against, so it proves the logic and not the coupling. A KaTeX + // upgrade that renamed `.katex` or the `KaTeX_` family prefix would leave it + // green while PDF export shipped a document with no maths fonts in it — and + // 0.18.0 renamed KaTeX's internal classes, so that upgrade is not + // hypothetical. + // + // This asks the installed KaTeX to render, and the installed stylesheet what + // that rendering needs. Nothing here is written down: rename anything and the + // intersection empties. + const katex = ((await import('katex')) as any).default; + const css = readSource('node_modules/katex/dist/katex.min.css'); + + const root = (globalThis as any).document.createElement('div'); + root.innerHTML = katex.renderToString('\\mathcal{L} = \\frac{1}{2}', { throwOnError: false }); + + const families = collectUsedKatexFamilies(root, css); + assert.ok( + families.size > 0, + 'the installed KaTeX renders markup the installed stylesheet gives no font to; ' + + 'PDF export would embed nothing and the maths would fall back to a UI font', + ); + assert.ok( + families.has('KaTeX_Main'), + `expected the base family among ${[...families].join(', ') || '(none)'}`, + ); + assert.ok( + katexFontUrlsToEmbed(css, families).length > 0, + 'families were found but no @font-face src to embed for them', + ); + }); }); describe('export sanitizing', () => { @@ -1236,6 +1283,45 @@ describe('exported theme', () => { assert.deepEqual(policy.get('media-src'), ['data:', 'https:', 'http:']); }); + test('every theme selector the stylesheet defines can be reproduced by an export', () => { + // Pins the two halves together: a new `:root[data-theme="…"]` block in + // styles.css must be a value the export is willing to write out, or that + // theme silently becomes unexportable. + const declared = new Set(); + appStylesheet.walkRules((rule) => { + for (const selector of rule.selectors) { + for (const match of selector.matchAll(/:root\[data-theme="([^"]+)"\]/g)) declared.add(match[1]); + } + }); + assert.ok(declared.size >= 2, 'expected styles.css to select themes by data-theme'); + + for (const theme of declared) { + assert.equal(exportThemeAttribute(theme), ` data-theme="${theme}"`, `${theme} must survive an export`); + assert.ok(matchesThemeSelector(build(theme), theme)); + } + }); + + test('the system theme is exported as "follow the reader", not as a frozen colour', () => { + // `system` is the one theme that is not a colour: MarkdownViewer deletes + // `data-theme` for it and lets `@media (prefers-color-scheme: dark)` decide. + // An export with no attribute reproduces that same cascade wherever it is + // opened, which is the instruction the user actually gave. Baking in the + // exporter's momentary system colour would make the file disagree with the + // author's own screen the next time their machine switches. + for (const absent of [undefined, null, '']) { + const doc = build(absent); + assert.doesNotMatch(htmlTag(doc), /data-theme/, `system theme must not pin a colour (${String(absent)})`); + assert.match(htmlTag(doc), /^$/); + } + + // And the mechanism it delegates to has to still be in the sheet that gets copied. + let followsSystem = false; + appStylesheet.walkAtRules('media', (atRule) => { + if (atRule.params === '(prefers-color-scheme: dark)' && atRule.some((node) => node.type === 'rule')) followsSystem = true; + }); + assert.ok(followsSystem, 'styles.css must keep a non-empty @media (prefers-color-scheme: dark) block'); + }); + test('the copied stylesheet and the article still land in the document', () => { const doc = build('dark', { styles: '.sentinel { color: red; }', articleHtml: '

hello

' }); assert.match(doc, /