Add render-diff and render-fixture subpath exports + MDX↔MDXish regression suite#1489
Draft
jamestclark wants to merge 8 commits into
Draft
Add render-diff and render-fixture subpath exports + MDX↔MDXish regression suite#1489jamestclark wants to merge 8 commits into
jamestclark wants to merge 8 commits into
Conversation
Engine-agnostic HTML diff tool published as its own subpath so
consumers can compare two rendered HTML strings without pulling in
the engine (compile/run/mdxish/renderMdxish).
Public API:
import { diff } from '@readme/markdown/render-diff';
import type { Change, DiffOptions, DiffResult, Preset, Severity }
from '@readme/markdown/render-diff';
diff(leftHtml, rightHtml, opts?) → { status: 'match' }
| { status: 'differ', severity, changes[] }
`leftHtml` and `rightHtml` are neutral param names so the same tool
serves MDX↔MDXish, before/after a @readme/markdown version bump, or
any other two-input comparison. Each `Change` carries `left`/`right`
fields matching the inputs.
Canonicalization pipeline (transplanted from the reference differ in
readmeio/readme PR #18479, re-targeted to htmlparser2/domhandler):
whitespace collapse, class sort, attribute normalization, noise-attr
drop, heading-counter strip, void-tag handling, text-equivalent
merging. Bottom-up SHA-1 content hash fast-path short-circuits to
match when both trees hash equal. Two presets: 'cross-engine' (default,
full normalization) and 'minimal' (skips span-flatten + adjacent-text-
merge — suitable for same-engine before/after).
Packaging:
- webpack serverConfig multi-entry adds 'render-diff' → dist/render-diff.node.js
- package.json exports map adds './render-diff' (types-first ordering)
- bundlewatch budget: 185 KB (actual 152 KB)
- scripts/verify-render-diff.cjs runs in CI via build:verify:
(1) dist-grep — bundle MUST NOT contain any of 5 engine identifiers
(2) self-ref probe — require('@readme/markdown/render-diff') resolves
- .github/workflows/ci.yml adds 'Verify subpath exports' step
Source-level guard (__tests__/lib/render-diff/no-engine-imports.test.ts)
prevents any file under lib/render-diff/ from importing engine modules
directly. Combined with the bundle-level dist-grep, this gives two
layers of defense against engine leakage.
Unit tests (__tests__/lib/render-diff/differ.test.ts) cover match
arms, differ arms, severity scoring, preset partition, fast-path,
and determinism.
Pairs with the engine-free render-diff subpath. Where render-diff
refuses to load engine code (152 KB), render-fixture deliberately
includes the full engine (4.04 MB) — its job is to render markdown
through MDX or MDXish for fixture-based regression testing.
Public API:
import { renderFixture, loadFixture } from '@readme/markdown/render-fixture';
import type { RenderContext, Engine, FixtureRenderResult }
from '@readme/markdown/render-fixture';
const { body, ctx } = loadFixture('./fixtures/install/');
const { html, error } = renderFixture(body, ctx, 'mdxish');
Motivating consumer: an `owl qa` style CLI that clones customer
projects and detects rendering regressions across @readme/markdown
version bumps. Pairing with `diff()` from the render-diff subpath
gives a complete regression loop in ~10 lines of consumer code.
`renderFixture` is deterministic: Date.now is frozen to 2024-01-01
and Math.random is replaced with a seeded LCG for the duration of
each render, then restored in a finally block. The helper asserts
sync return — if any of compile/run/mdxish/renderMdxish becomes
async in the future, the determinism guarantee breaks loudly rather
than silently producing flaky output.
Packaging:
- webpack serverConfig multi-entry adds 'render-fixture' → dist/render-fixture.node.js
- package.json exports map adds './render-fixture' (types-first ordering)
- bundlewatch budget: 4.5 MB (actual 4.04 MB)
- verify-exports script extends to also run scripts/verify-render-fixture.cjs:
(1) positive-control: bundle MUST contain all 5 engine identifiers
(opposite of render-diff's negative check)
(2) self-ref probe: require('@readme/markdown/render-fixture') resolves
and both engines successfully render a smoke-test markdown body
PKG-04 (render-diff engine-free) constraint preserved unchanged — its
check still runs and still passes. render-fixture intentionally
violates that constraint because it has to.
Two test suites that share a fixture corpus:
- Suite A (__tests__/regression/snapshots.test.ts) renders each
fixture through both engines separately and toMatchSnapshot()s
the raw HTML — one snapshot entry per (fixture, engine), 26 total.
Byte-level changes in either engine surface as a failing snapshot
on the PR that caused them.
- Suite B (__tests__/regression/equivalence.test.ts) renders each
fixture through both engines, runs diff() on the two outputs, and
toMatchSnapshot()s the structured diff. As MDX↔MDXish parity
improves, the snapshot shrinks toward { status: 'match' }.
Both suites auto-discover fixture directories via readdirSync — adding
a new fixture takes zero code change.
Fixture format (documented in __tests__/regression/fixtures/README.md):
- body.md — markdown source
- context.json — variables + glossary (engine inputs)
- components/*.mdx — optional custom components by tag name
Fixture corpus (13 directories):
Baselines (the two arms of DiffResult + the data-plumbing surface):
- convergent — match branch (plain markdown)
- divergent — differ branch (HTMLBlock safeMode)
- rich — variables + glossary + custom component
Regression fixtures grounded in merged bug fixes:
- tables-with-html — PR #1466, #1467, #1463, #1469, #1471
- magic-blocks-table — PR #1451, #1452
- unclosed-tags — PR #1474, #1477, #1480, #1482
- callouts-and-glossary — PR #1408, #1421, #1441, #1454
- jsx-attribute-entities — PR #1461, #1462
- variables-everywhere — PR #1423, #1459, #1471
- embeds — PR #1476
- consecutive-emojis-fa — PR #1390, #1416, #1421, #1449
- compact-headings — PR #1428
- htmlblock-with-script — PR #1457 (scoped to non-runScripts path)
Each fixture README documents what it covers and what regressing it
would mean. Three regression fixtures intentionally render empty on
the MDX side (magic-blocks-table, unclosed-tags, variables-everywhere)
because they exercise MDXish-flavored syntax strict MDX rejects — the
MDXish snapshot is the regression contract for those.
39 snapshot entries committed: 26 in snapshots.test.ts.snap (Suite A)
+ 13 in equivalence.test.ts.snap (Suite B).
The Change.kind values emitted by alignChildren were the reverse of what types.ts documents: - 'missing' should mean present in left, absent in right - 'extra' should mean present in right, absent in left The differ emitted the opposite at every call site, with the helper's left/right field placement compensating so the data was self-consistent but the public discriminant was wrong. Tightens the differ.test.ts case to assert kind strictly (it previously accepted either label) and adds the sibling test for the missing case. Regenerates the equivalence suite snapshots (all 30 entries flip from missing → extra; field shapes unchanged). Also folds in the pre-staged "Suite B:" describe rename so the regenerated snapshot keys match the test names.
…ns tests" Matches the parallel rename of Suite B → "MDX↔MDXish equivalence" and refreshes the snapshot keys accordingly.
The new exports block introduced in a665b8a inadvertently narrowed the package's import surface: without exports, Node falls back to filesystem lookup so any deep import works; with exports, every path not listed throws ERR_PACKAGE_PATH_NOT_EXPORTED. The previous block only listed '.', './render-diff', and './render-fixture', which would have broken existing consumers doing imports like '@readme/markdown/styles/main.css' or '@readme/markdown/components/...'. Adds an explicit './package.json' entry (commonly read by tooling) and a './*' → './*' wildcard fallback. Specific entries still take precedence in Node's resolver, so the new subpath exports continue to hit their bundled .node.js targets.
…tution
renderFixture previously passed variables only to renderMdxish(), which
runs after the MDXish parse step. That meant variablesCodeResolver — the
plugin in mdxish() that resolves <<...>> and {user.*} inside code and
inline-code nodes — saw `variables: undefined` and fell back to uppercase
placeholders (APIKEY, USER.REGION). So MDXish-side fixture snapshots
locked in the unsubstituted state, even though variables-everywhere is
the dedicated fixture for variable-substitution regressions (PRs #1423,
#1459, #1471).
Pass ctx.variables to both mdxish() and renderMdxish() so the regression
suite exercises real end-to-end substitution. Re-record snapshots:
- rich (mdxish) and variables-everywhere (mdxish) now resolve apiKey to
sk_test_abc123 and region to us-east-1 inside code.
- The equivalence snapshot for `rich` now correctly reports the
MDX↔MDXish engine divergence on code-block substitution (MDX treats
code as opaque; MDXish runs variablesCodeResolver). That divergence
is real — captured rather than masked.
- Variables without configured values (e.g. `<<name>>`) still fall back
to uppercase, as the resolver intends.
Update the variables-everywhere README: the "Known wiring gap" section
is replaced with a "Substitution coverage" section documenting the new
end-to-end coverage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 What does this PR do?
Adds two new subpath exports plus a 13-fixture regression suite that gates any future rendering change in the
@readme/markdownengine.New public surface
The two subpaths intentionally split along the engine boundary so a consumer that only wants to diff two HTML strings (e.g. CI gating on rendering parity across
@readme/markdownversion bumps) doesn't have to pull in the full 4 MB engine bundle. Pairing them gives a complete render-and-compare loop in ~10 lines of consumer code:Why the diff tool is engine-agnostic
The public API uses neutral
left/rightnaming (diff(leftHtml, rightHtml, opts),Change.left/Change.right), notmdx/mdxish. Suite B in this PR uses it for MDX↔MDXish comparison; other consumers use the same tool for before/after a version bump or any two-input HTML comparison. The'minimal'preset is explicitly designed for same-engine before/after diffing (skips span-flatten + adjacent-text-merge that would mask real structural changes).'cross-engine'(the default) does the fuller normalization needed for MDX↔MDXish.Regression suite — 13 fixtures
Two suites share the fixture corpus:
__tests__/regression/snapshots.test.ts) renders each fixture through both engines separately andtoMatchSnapshot()s the raw HTML. 26 entries total (13 fixtures × 2 engines). Byte-level changes in either engine surface as a failing snapshot on the PR that caused them.__tests__/regression/equivalence.test.ts) renders each fixture through both engines, runsdiff()on the two outputs, andtoMatchSnapshot()s the structured diff. As MDX↔MDXish parity improves, snapshots shrink toward{ status: 'match' }.Both suites auto-discover fixture directories via
readdirSync— adding a new fixture takes zero code change. The fixture format (body.md+context.json+ optionalcomponents/*.mdx) is documented in__tests__/regression/fixtures/README.md.The 13 fixtures break into baselines and regression coverage:
Baselines (the two arms of
DiffResult+ the data-plumbing surface):convergent— match branch (plain markdown)divergent— differ branch (HTMLBlock safeMode)rich— variables + glossary + custom componentRegression fixtures (each grounded in a real merged bug fix from the past few months):
tables-with-html(PR fix: code elements stripped from td elements #1466, fix: don't parse content of <code> elements in table cells #1467, fix(mdxish): preserve attributes on raw <table> rows/cells #1463, fix(mdxish): keep md images inline when in tableCells #1469, fix: dont normalize emphasis syntax within html code elements #1471)magic-blocks-table(PR fix(mdxish): multiple consecutive magic blocks under a list item not rendering & out of order #1451, fix(mdxish): consecutive magic blocks not rendering if they have html tags in their content #1452)unclosed-tags(PR fix(mdxish): bail orphaned<Tag>openers before they eat sibling blocks #1474, fix(mdxish): preserve raw-content HTML tags when unclosed on a line #1477, fix(mdxish): stop orphan closer tags from breaking table parsing #1480, fix(mdxish): handle orphan void closers in JSX table cells #1482)callouts-and-glossary(PR fix: guard against empty Glossary tag causing crash #1408, fix(mdxish): fix FA Emojis in Callouts #1421, fix(mdxish): legacy glossary syntax in callout title crashing #1441, fix(Callout): only apply callout title style to headings #1454)jsx-attribute-entities(PR fix(mdxish): render Image captions containing entity-encoded JSX #1461, fix(mdxish): decode html entities in jsx attribute values #1462)variables-everywhere(PR fix(mdxish): handle user vars on standalone lines in tables #1423, fix(mdxish): skip variable resolution in Mermaid code blocks #1459, fix: dont normalize emphasis syntax within html code elements #1471)embeds(PR fix: render iframe/youtube/pdf/jsfiddle embeds without embedly html #1476)consecutive-emojis-fa(PR fix(mdxish): allow rendering consecutive emojis #1390, fix(mdxish): add gemoji support to api-header title parser #1416, fix(mdxish): fix FA Emojis in Callouts #1421, fix(mdxish): gemoji & expressions not rendering in components #1449)compact-headings(PR fix(mdxish): Add compact heading preprocessor #1428)htmlblock-with-script(PR fix(mdxish): scope \n unescaping in HTMLBlock content #1457 — scoped to non-runScriptspath)Each fixture has its own README documenting what it covers and what regressing it would mean.
Build verification
npm run build:verifyruns both subpath check scripts in CI:scripts/verify-render-diff.cjs— dist-grep ensuresdist/render-diff.node.jscontains none of 5 engine identifiers (bundle isolation); self-ref probe confirms the subpath resolves anddiff()worksscripts/verify-render-fixture.cjs— positive control ensuresdist/render-fixture.node.jscontains all 5 engine identifiers (would-be-broken if missing); self-ref probe confirms both engines render via the subpath__tests__/lib/render-diff/no-engine-imports.test.tsprevents any file underlib/render-diff/from importing engine modules directly (defense-in-depth for the dist-grep)Commit-by-commit
feat(render-diff)feat(render-fixture)test(regression)Each commit builds and verifies in isolation (bisectable).
🧪 QA tips
Specifically:
dist/render-diff.node.jsis ~152 KB and contains no engine identifiers (grep mdxishAstProcessor dist/render-diff.node.jsshould be empty)dist/render-fixture.node.jsis ~4.04 MB and contains all engine identifiers (grep -c mdxishAstProcessor dist/render-fixture.node.jsshould be > 0)node -e "console.log(require('@readme/markdown/render-diff').diff('<p>x</p>', '<p>x</p>'))"prints{ status: 'match' }node -e "const {renderFixture} = require('@readme/markdown/render-fixture'); console.log(renderFixture('# hi', { variables: { defaults: [], user: {} }, glossary: [], components: [] }, 'mdxish').html.includes('hi'))"printstrue__tests__/regression/fixtures/is auto-discovered by both suites on the nextnpm testrunnpm testsuite (full run: 1059 pass on this branch)📸 Screenshot or Loom
N/A — no user-visible UI change. The new surface is a pair of programmatic subpath exports and a CI test suite.