Skip to content

autodoc: ensure unique example storage with duplicate titles#71

Open
becky-gilbert wants to merge 11 commits into
autodocfrom
example-naming
Open

autodoc: ensure unique example storage with duplicate titles#71
becky-gilbert wants to merge 11 commits into
autodocfrom
example-naming

Conversation

@becky-gilbert

@becky-gilbert becky-gilbert commented Jul 16, 2026

Copy link
Copy Markdown

Fixes #61

This PR contains changes to address the problem of multiple examples having the same title and overwriting one another when collected, which was caused by keying the examples by title (which may collide) in the example storage data structure. This PR solves the issue by using the example file path (always unique) as the example identifier/key.

There are some other changes here that are related to the core bug fix but definitely subjective, so just let me know if you want me to remove any of those things.

Source code

src/parsers/utils.ts

  • collectExamples now keys the examples by the file path returned by getExampleInfo, which is always unique, instead of its title, which could collide. And the value object for these examples now includes title, hasCustomTitle, and displayPath alongside path and code (see related changes to ExampleInfo type below).
  • getExampleInfo return type changed from:
    Record<string, ExampleInfo> | undefined
    to
    ExampleInfo | undefined
    so that one file always produces exactly one example object result. (Previously, getExampleInfo always returned an object with exactly one key-value pair for a single example file, so the Record wrapper was unnecessary. The key-value handling for multiple examples is handled by collectExamples).
  • collectExamples now scans directories recursively (previously flat only)
  • displayPath is computed as path relative to the examples root for directory scans, or just the filename for single-file input
  • If the examples have duplicate sentinel titles, they will get a numbered suffix ("(2)", "(3)", etc.) and will produce a console warning. Duplicate <title> tag titles are left as-is because the displayPath in the heading handles uniqueness at render time.
  • Wrapped the collectExamples file iteration and title logic in try/catch so that a single bad example file (e.g. no title tag or sentinel) in a directory doesn't crash the whole run. (If the example path is a single file rather than a directory, it will still throw as before.)

src/renderers/extension.ts / plugin.ts / timeline.ts

  • Updated the section heading logic from:
    ### ${title} (${path}) (always show file path)
    to
    ### ${title} (no path when hasCustomTitle is true), or
    ### ${title} (${displayPath}) (same as original when hasCustomTitle is false)
    This way, custom sentinel titles are treated as intentional and complete, and the user can use this method to remove the file path from the example's section title. Whereas the use of the <title> tag is treated as the default case and not always expected to be unique. (Note that duplicate sentinel titles are still handled but that's done by the parser)

src/types/info.ts

Adds the following to ExampleInfo type:

  • title: moved out of the record key into the value
  • hasCustomTitle: whether the title came from the jspsych-autodoc:title sentinel (vs <title> tag)
  • displayPath: the path shown in section headings, relative to the examples root directory

Tests

New tests and test files

tests/parsers/extension.test.ts

  • Duplicate sentinels - Both examples are kept, the second title gets renamed to "my example (2)" and produces a warning.
  • Duplicate <title> tag titles in the same directory - Both examples are kept, the titles are not renamed (displayPath handles uniqueness at render time), and no warning is emitted.
  • Duplicate <title> tag titles with same file name in different subdirectories - Both examples are kept and their displayPath values include the subdirectory to ensure uniqueness.

tests/renderers/extension.test.ts

  • Subdirectory paths in headings - The rendered section headings include the full subdirectory-relative path (path1/duplicated_filename.html) not just the bare filename (duplicated_filename.html).

New fixture files

  • title-sentinel-collision-tests with file-a.html and file-b.html - both with the same jspsych-autodoc:title sentinel
  • title-tag-collision-tests with file-a.html and file-b.html - both with the same <title> tag
  • title-filepath-collision-tests with /path1/duplicated_filename.html and path2/duplicated_filename.html - same filename with <title> tags in different subdirectories

Changes to existing tests and test files

Parser tests

  • Example object lookups changed to use file path as key instead of title
  • .title assertions added with the existing .path and .code assertions (since title is now a value field rather than the key)

Renderer tests:

  • Update assertion for basic example title (because it has hasCustomTitle: true so the path no longer appears in the rendered heading)

Renderer fixtures (info objects):

  • Update examples object to use the file path rather than title as key
  • Update value with title, hasCustomTitle: true, and displayPath fields

Snapshots

  • Updated the basic example title (because the path is no longer appended)

…: getExampleInfo now just returns info object (no top-level file/example key) and adds hasCustomTitle flag; collectExamples keys example info by the file path (always unique), sets display path for title (default titles only), checks for and handles duplicate titles (warning, adds number to prevent overwritting), puts everything in try/catch
…e keyed by file path (always unique), adds title, hasCustomTitle, and displayPath to example value object
…or same title and file name but different paths
@changeset-bot

changeset-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a84c064

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

…ath rather than title as key for accessing stored examples
…le, and only skip file if examplesPath is a directory
@becky-gilbert becky-gilbert changed the title autodoc: ensure unique example section names autodoc: ensure unique example storage with duplicate titles Jul 16, 2026
@becky-gilbert
becky-gilbert requested a review from jadeddelta July 16, 2026 22:33

@jadeddelta jadeddelta 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.

just some things to address with handling different OS file paths

);
isDirectory = true;
const collectHtml = (dir: string) => {
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

from what I understand, fs.readdirSync is unsorted (implementation differs across OSes) so whichever one of shared titles becoming the secondary one might get flipped. would be good to sort the array before looping over it

try {
const info = getExampleInfo(file, inferFallback);
if (info) {
info.displayPath = isDirectory ? path.relative(examplePath, info.path) : path.basename(info.path);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

not sure if this ensures if paths could persist across OSes, thinking about windows using backslash (darn windows..), consider either using relativizeExamplePaths in the CLI utils.ts, or .split(path.sep).join("/") and then remove the relativizeExamplePaths call in the runCli function in cli.ts.

const baseTitle = info.title;
const count = titleCounts.get(baseTitle) ?? 0;
titleCounts.set(baseTitle, count + 1);
if (count > 0) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

it looks like this might not work for titles "Hello", "Hello (2)", "Hello", the paths are stored separately so it won't be clobbered but it will result in duplicate section headers once rendered out. not sure if worth doing this edge case because it's not even that big of a deal, just flagging it

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.

2 participants