Skip to content

publish.mjs: keep every --pages page when two share a theme - #121

Open
juliasadler wants to merge 1 commit into
wheresryan22:mainfrom
juliasadler:fix/pages-same-theme
Open

juliasadler wants to merge 1 commit into
wheresryan22:mainfrom
juliasadler:fix/pages-same-theme

Conversation

@juliasadler

@juliasadler juliasadler commented Oct 17, 2026 •

Copy link
Copy Markdown

What this changes

  • detectPages in scripts/publish.mjs: when two --pages entries resolve to the same key (same data-theme, or the same filename slug), the later entry now falls back to its filename slug — with a numeric suffix if that is taken too — instead of silently overwriting the earlier one. Both pages end up recorded under distinct keys.
  • The inline slug expression is now a slugOf helper, plus a freeSlug helper that finds an unclaimed key. No code comments, per repo rule.
  • Regression test in scripts/publish.test.mjs with a two-same-theme fixture, as suggested in the issue.
  • CHANGELOG entry under [Unreleased] → Fixed.

Why

Fixes #89. Before, --pages main.html,alt.html with both files at data-theme="dark" produced figure.pages = { "dark": "src/alt.html" }: main.html vanished as a page and shipped as a dead file under dest/src. After, it produces { "dark": "src/main.html", "alt": "src/alt.html" }.

I took the issue's first suggested direction (slug fallback) rather than throwing: it records both pages, as the issue's "Expected" section asks, and it doesn't turn a previously silent success into a hard error.

Checks

  • Every example rebuilds, and git status --short shows only the changes I meant. — git status --short shows only CHANGELOG.md, scripts/publish.mjs, scripts/publish.test.mjs. No example touched, so no figure rebuild applies; node --test scripts/publish.test.mjs passes 8/8 including the new regression test.
  • No comments in code.
  • Kit changes are opt-in, and every existing example is byte-identical — N/A, no kit change.
  • I built and looked at both themes. — N/A, no visual change (publish script only).
  • Figures with pipes, cables or round parts … — N/A.
  • node scripts/lines.mjs … — N/A.
  • Shaders … — N/A.
  • 3D … — N/A.
  • Every number in the figure … is true. — N/A, no figure numbers changed.
  • A line under Unreleased in CHANGELOG.md.

Screenshots

N/A — no visual change.

@rahul05ranjan rahul05ranjan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean fix for a real footgun — the old pages[...] = page silently ate the first page whenever two shared a theme, and the recording would just point at the later file.

I traced the fallback chain and it holds together: first same-theme page keeps the theme key, the colliding one drops to its filename slug, and a slug that's also taken gets -2, -3, … via freeSlug, so a third dark page or an unlucky slug collision can't silently overwrite either. Object.hasOwn over in is the right call too, since it ignores prototype keys.

One pre-existing edge this inherits rather than introduces: a page literally named dark.html with no theme slugs to the key dark, which consumers likely treat as the dark-theme entry. Before your change it collided head-on; now it survives as dark-2 if a themed page got there first, which is better but still slightly ambiguous. Probably not worth solving unless someone hits it — the numeric suffix at least makes the two pages distinct now.

The test covers the real case well, including the assertion that the overwritten page no longer ships as a stray source file — that's the user-visible half of the bug, easy to forget.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

publish.mjs --pages silently drops a page when two listed pages share the same theme

2 participants