Ready

mirendev/runtime#1214

This is still a draft, so I'm judging whether it's ready for human review. The design is solid: a postBuild Docusaurus plugin that renders per-page social cards with Satori + resvg, quantizes them with UPNG for size, caches them in node_modules/.cache keyed by a content hash, and a swizzled DocItem/Metadata component that points og:image at the right path. The separation of the path-building logic into path.ts — shared between the plugin and the theme component — is exactly the right call, and the CI cache setup (hash-keyed save, prefix-keyed restore) is clean.

A few things I want to flag before this goes to human review:

1. doc.description can be undefined, and .update() will throw.

@docusaurus/plugin-content-docs types DocMetadata.description as string | undefined. At line 156, createHash().update(doc.description) will throw if any doc lacks a description, because Node's Hash.update rejects undefined. At line 161 the same undefined would be passed into card.description and rendered by Satori as-is (which may silently produce an empty string or throw). Both call sites should guard with ?? ''.

2. __filename is a compiled .js path, not the source .ts path.

The template hash at line 142 reads readFileSync(__filename) to detect changes to the plugin. When jiti transpiles the file, __filename will point at a temporary compiled location (or the .ts source under jiti), but more importantly, during a normal build Docusaurus loads the config through ts-node/jiti — the exact path depends on the runtime. If __filename ever resolves to the transpiled output rather than the source file, two developers may compute different hashes for the same logical file, silently invalidating each other's caches. The hash over the font files already covers most drift; skipping the self-hash avoids this ambiguity entirely, at the cost of requiring a manual cache key bump when the template changes (which the GH Actions step already does via hashFiles('docs/plugins/og-images/**')).

3. No tests for ogImagePath.

path.ts is a pure function with a few tricky cases (/index.png, trailing-slash stripping). It's the single piece of logic shared between the builder and the consumer, so a bug there silently breaks the link between the written file and the og:image tag. A couple of unit test cases would give good confidence.

These are real risks (especially #1, which can cause a build failure for any doc that has no description), so I'd want them addressed before marking this ready — but the overall shape of the implementation is good.

Inline comments


Verdict: caveats

Data

{
  "owner": "mirendev",
  "repo": "runtime",
  "number": 1214,
  "verdict": "caveats",
  "event": "comment",
  "summary": "This is still a draft, so I'm judging whether it's ready for human review. The design is solid: a `postBuild` Docusaurus plugin that renders per-page social cards with Satori + resvg, quantizes them with UPNG for size, caches them in `node_modules/.cache` keyed by a content hash, and a swizzled `DocItem/Metadata` component that points `og:image` at the right path. The separation of the path-building logic into `path.ts` — shared between the plugin and the theme component — is exactly the right call, and the CI cache setup (hash-keyed save, prefix-keyed restore) is clean.\n\nA few things I want to flag before this goes to human review:\n\n**1. `doc.description` can be `undefined`, and `.update()` will throw.**\n\n`@docusaurus/plugin-content-docs` types `DocMetadata.description` as `string | undefined`. At line 156, `createHash().update(doc.description)` will throw if any doc lacks a description, because Node's `Hash.update` rejects `undefined`. At line 161 the same `undefined` would be passed into `card.description` and rendered by Satori as-is (which may silently produce an empty string or throw). Both call sites should guard with `?? ''`.\n\n**2. `__filename` is a compiled `.js` path, not the source `.ts` path.**\n\nThe template hash at line 142 reads `readFileSync(__filename)` to detect changes to the plugin. When jiti transpiles the file, `__filename` will point at a temporary compiled location (or the `.ts` source under jiti), but more importantly, during a normal build Docusaurus loads the config through `ts-node`/`jiti` — the exact path depends on the runtime. If `__filename` ever resolves to the transpiled output rather than the source file, two developers may compute different hashes for the same logical file, silently invalidating each other's caches. The hash over the font files already covers most drift; skipping the self-hash avoids this ambiguity entirely, at the cost of requiring a manual cache key bump when the template changes (which the GH Actions step already does via `hashFiles('docs/plugins/og-images/**')`).\n\n**3. No tests for `ogImagePath`.**\n\n`path.ts` is a pure function with a few tricky cases (`/` → `index.png`, trailing-slash stripping). It's the single piece of logic shared between the builder and the consumer, so a bug there silently breaks the link between the written file and the `og:image` tag. A couple of unit test cases would give good confidence.\n\nThese are real risks (especially #1, which can cause a build failure for any doc that has no description), so I'd want them addressed before marking this ready — but the overall shape of the implementation is good.",
  "comments": [
    {
      "path": "docs/plugins/og-images/index.ts",
      "line": 156,
      "side": "RIGHT",
      "body": "`Hash.update()` rejects `undefined`, and `DocMetadata.description` is typed as `string | undefined`. Any doc without a description will throw here and abort the entire build. Change this to `.update(doc.description ?? '')` (and the matching call at line 161 where it's passed to `renderCard`).",
      "ai_prompt": "In docs/plugins/og-images/index.ts, line 156 calls `.update(doc.description)` and line 161 passes `doc.description` to renderCard. DocMetadata.description is typed as string | undefined. Node's Hash.update throws when given undefined, and Satori may also misbehave. Change line 156 to `.update(doc.description ?? '')` and line 161 to `description: doc.description ?? ''` to guard against docs that lack a description."
    },
    {
      "path": "docs/plugins/og-images/index.ts",
      "line": 142,
      "side": "RIGHT",
      "body": "`__filename` under jiti / ts-node may point to a transpiled artifact rather than the `.ts` source, meaning the file content read here isn't stable across environments and could produce different hashes for the same logical code. The GH Actions cache key already invalidates the cache when any file under `docs/plugins/og-images/**` changes (via `hashFiles`), so this self-hash is redundant and introduces the ambiguity. Consider removing it and relying solely on the font and logo hashes (plus the CI key) to detect plugin changes.",
      "ai_prompt": "In docs/plugins/og-images/index.ts around lines 141-145, the template hash includes readFileSync(__filename). Under jiti or ts-node, __filename may point to a transpiled file rather than the source, producing environment-dependent hashes for the same code. The GH Actions cache key already covers plugin-file changes via hashFiles. Remove the .update(readFileSync(__filename)) call so the template hash only covers the fonts and logo, which are stable binary assets."
    }
  ],
  "posted_to_pr": true,
  "draft": true
}