Add ChildrenTypeRegistry so consumers type mdx children once per project#138
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
📜 Recent review details
|
| Layer / File(s) | Summary |
|---|---|
ChildrenTypeRegistry hook and TagChildren application packages/leadtype/src/mdx/tag-types.ts |
Introduces ChildrenTypeRegistry augmentation interface and TagChildren conditional type, replacing children?: unknown with children?: TagChildren across Callout, Tabs/Tab, Steps/Step, Mermaid, Accordion, Cards/Card, FileTree/Folder, Audience, Prompt, Example, Section, and Details prop types. |
Public export wiring and default test packages/leadtype/src/mdx/index.ts, packages/leadtype/src/mdx/tag-types.test.ts |
Re-exports ChildrenTypeRegistry and TagChildren from the package entry point and adds a Vitest module verifying TagChildren and CalloutProps["children"] default to unknown when no augmentation is registered. |
Documentation and lockfile updates .changeset/mdx-children-registry.md, docs/integrations/integrate-with-fumadocs.mdx, docs/reference/mdx.mdx, docs/paths.lock.json |
Adds a changeset describing the new hook, updates the Fumadocs and mdx reference docs to demonstrate the register-once types.d.ts pattern instead of casting children as ReactNode, and refreshes related lockfile hashes. |
Estimated code review effort: 2 (Simple) | ~15 minutes
Possibly related PRs
- inthhq/leadtype#137: Directly overlaps with the Fumadocs
Calloutchildren example, which this PR updates to useChildrenTypeRegistryaugmentation instead of a cast.
Poem
A rabbit hopped through types unknown,
Then registeredchildrenas its own.
No casts today, just clean JSX to see,
A tiny hook for typey harmony. 🐇
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly summarizes the main change: adding a registry so MDX children can be typed once per project. |
| Description check | ✅ Passed | The description is directly related to the changeset and accurately describes the registry, docs, and type updates. |
| Linked Issues check | ✅ Passed | The PR addresses #136 by replacing per-component casts with an opt-in ChildrenTypeRegistry while preserving unknown by default. |
| Out of Scope Changes check | ✅ Passed | The changes stay within the MDX children-typing objective; the tests, docs, and lockfile updates are supporting edits. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
KayleeWilliams/mdx-children-registry
Comment @coderabbitai help to get the list of available commands.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one observation worth confirming below.
Reviewed changes — a type-level registry that lets consumers set the children type for every leadtype/mdx tag prop once per project, replacing per-component casts, with no runtime and no renderer dependency added.
- Add
ChildrenTypeRegistry+TagChildren— empty augmentation-hook interface plus a conditional type (ChildrenTypeRegistry extends { type: infer T } ? T : unknown) intag-types.ts; both re-exported fromsrc/mdx/index.ts. - Convert 17
children?: unknownfields tochildren?: TagChildren— every tag prop type now sources its child type from the registry;FilePropsis correctly left alone (no children). - Add type-level test —
tag-types.test.tsassertschildrenstaysunknownwhen nothing is registered, locking in the non-breaking default. - Docs adopt the registry —
reference/mdx.mdx(React) andintegrate-with-fumadocs.mdxdrop thechildren as ReactNodecast /Omit<…,"children">wrapper in favor of the augmentation; Vue/Svelte/Solid/Astro examples keep the framework-appropriateOmitpattern. - Changeset —
minorbump.
The non-breaking claim holds: with no augmentation TagChildren resolves to unknown, so children?: TagChildren is behaviorally identical to the prior children?: unknown, and the new type test guards it.
ℹ️ paths.lock.json updates hashes for two pages this PR doesn't touch
The lockfile bumps body-content hashes for framework-matrix and deploy-generated-artifacts in addition to the two docs pages actually edited here. Neither page renders MDX tag-prop content (framework-matrix is a static routing table), and because TagChildren resolves to unknown by default, this change produces no visible difference in their generated output — so the extra churn most likely reflects pre-existing lockfile drift being corrected by regeneration rather than a real content change.
There's no functional risk (redirect rename-detection keys off path removal, not a hash change on a stable path), but it's worth a glance to confirm the churn is intended rather than an artifact of regenerating against unrelated local state.
Technical details
# paths.lock.json hash churn on untouched pages
## Affected sites
- docs/paths.lock.json — hash changes for `/docs/integrations/framework-matrix` and `/docs/pipeline/deploy-generated-artifacts`, neither of which is in this PR's diff.
- Hash source: `hashRedirectContent` in `packages/leadtype/src/redirects/node.ts` = `sha256(parseFrontmatter(markdown).content.trim())` — pure page-body hash, frontmatter excluded.
## Required outcome
- Confirm the two extra hash changes are intended (genuine content dependency or an accepted drift correction) and not a side effect of regenerating the lockfile against unrelated local/base state.
## Suggested approach (optional)
- Regenerate the lockfile from a clean `main` + this branch's docs edits only, and verify the diff reduces to `integrate-with-fumadocs` + `reference/mdx`. If the two extra hashes reappear, they're a pre-existing drift on `main` worth a separate note.Claude Opus ｜ 𝕏
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98d3657ee8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/leadtype/src/mdx/tag-types.test.ts`:
- Around line 6-10: The current IsUnknown<T> helper in tag-types.test.ts is too
permissive because it treats any as unknown, so tighten the helper to explicitly
exclude any before checking for unknown. Update the test around TagChildren to
use a stricter type guard in IsUnknown and keep the assertion focused on the
default type so the test fails if TagChildren regresses from unknown to any.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ef9c794e-4011-42d0-b190-f2262ed30407
📒 Files selected for processing (7)
.changeset/mdx-children-registry.mddocs/integrations/integrate-with-fumadocs.mdxdocs/paths.lock.jsondocs/reference/mdx.mdxpackages/leadtype/src/mdx/index.tspackages/leadtype/src/mdx/tag-types.test.tspackages/leadtype/src/mdx/tag-types.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
packages/leadtype/src/mdx/tag-types.test.tspackages/leadtype/src/mdx/index.tspackages/leadtype/src/mdx/tag-types.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Use descriptive names for functions, variables, and types for meaningful naming
Add comments for complex logic, but prefer self-documenting code
Files:
packages/leadtype/src/mdx/tag-types.test.tspackages/leadtype/src/mdx/index.tspackages/leadtype/src/mdx/tag-types.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: Write assertions insideit()ortest()blocks
Avoid done callbacks in async tests - use async/await instead
Don't use.onlyor.skipin committed code
Keep test suites reasonably flat - avoid excessivedescribenesting
Files:
packages/leadtype/src/mdx/tag-types.test.ts
**/index.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid barrel files (index files that re-export everything)
Files:
packages/leadtype/src/mdx/index.ts
🧠 Learnings (1)
📚 Learning: 2026-06-09T18:30:08.038Z
Learnt from: KayleeWilliams
Repo: inthhq/leadtype PR: 97
File: .changeset/search-prototype-safety-and-scaling.md:5-5
Timestamp: 2026-06-09T18:30:08.038Z
Learning: In this repo, `.changeset/*.md` files must not start the body with an H1/first-line heading (`#`) immediately after the YAML frontmatter. The changesets tool inlines the body as bullet entries into `CHANGELOG.md` during release, and a leading `#` heading would break the generated changelog format. As a result, MD041 (`first-line-heading`) warnings for files under `.changeset/` are expected false positives and should be ignored.
Applied to files:
.changeset/mdx-children-registry.md
🪛 markdownlint-cli2 (0.22.1)
.changeset/mdx-children-registry.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (11)
packages/leadtype/src/mdx/tag-types.ts (4)
60-67: Conditional-type fallback logic is correct.
ChildrenTypeRegistry extends { type: infer T } ? T : unknowncorrectly resolves tounknownwhen the interface is empty (structurally,{}isn't assignable to{ type: infer T }) and to the registered type once a consumer merges atypemember in. Logic itself is sound, independent of the re-export concern raised above.
6-20: LGTM!
95-95: LGTM!Also applies to: 109-129, 232-254, 267-267, 277-284, 299-299, 361-361, 383-383, 393-393, 402-402
42-67: 🗄️ Data Integrity & IntegrationAdd an augmentation test for
leadtype/mdxThe test suite should exercise
declare module "leadtype/mdx"and assertTagChildrenresolves to the registered child type, not just the defaultunknownpath.packages/leadtype/src/mdx/index.ts (1)
78-78: LGTM! (Cross-referenced concern about module-augmentation semantics forChildrenTypeRegistryis raised at its declaration site intag-types.ts.)Also applies to: 98-98
.changeset/mdx-children-registry.md (2)
5-5: 📐 Maintainability & Code QualityMarkdownlint MD041 warning is a known false positive.
Based on learnings, changeset files in this repo intentionally omit a leading heading since the changesets tool inlines the body into
CHANGELOG.md; MD041 warnings for.changeset/*.mdare expected and should be ignored.Source: Learnings
1-25: LGTM!docs/integrations/integrate-with-fumadocs.mdx (1)
96-121: LGTM!docs/reference/mdx.mdx (1)
49-94: LGTM!docs/paths.lock.json (2)
53-58: LGTM!Also applies to: 148-151
82-82: 🗄️ Data Integrity & IntegrationCheck the
/docs/pipeline/deploy-generated-artifactslockfile hash. The updated hash looks outside this PR’s scoped docs changes; if that page wasn’t edited here, remove the stale lockfile entry.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the single new commit tightens the type-level test helper so it can no longer be satisfied by an any regression.
- Harden
IsUnknownagainstany— addsIsAny<T> = 0 extends 1 & T(the standardany-detection idiom) and reordersIsUnknown<T>to rejectanybefore theunknown extends Tcheck. The default-children test now fails ifTagChildrenever regresses fromunknowntoany, closing the false-green gap in the prior helper.
Verified the helper resolves correctly: unknown → true, any → false, string → false. The registry logic, tag-prop conversions, and docs from earlier commits are unchanged.
Claude Opus ｜ 𝕏
KayleeWilliams
force-pushed
the
KayleeWilliams/mdx-children-registry
branch
from
6228e1f to
6a411f1
Compare
1 participant
Summary
Closes #136 — resolves the
children as ReactNodepaper cut the snippet typechecker surfaced, with the registry-augmentation shape.New
ChildrenTypeRegistryinterface (empty by default) +TagChildrenresolved type exported fromleadtype/mdx; all 17children?: unknownfields across the tag prop contracts becomechildren?: TagChildren. Non-breaking: with no augmentation,TagChildrenresolves tounknown— exactly the previous behavior (guarded by a type-level test).Consumers register their renderer's child type once per project:
and every component gets typed
childrenwith no casts — leadtype still ships zero renderer dependencies (the core/adapter boundary test even flagged a React mention in a source docblock during development; the in-source examples are framework-neutral, the copy-pasteable React version lives in the docs).Verified the augmentation merges through the rolled-up dist types with a scratch consumer asserting a real type error appears once
stringis registered.Docs updated:
reference/mdxtyping guidance leads with the registry, and the Fumadocs integration example uses the augmentation instead of the cast — so our own snippet typechecker validates the pattern in the exact consumer scenario on every lint run.Validation
dist/mdx/index.d.ts(@ts-expect-errorheld oncestringwas registered)leadtype lint(snippet typecheck) — all 51 files pass with the new example shapesbun test packages/leadtype+check-types+ ultracite clean; core/adapter boundary test greenbun testsuite passed🤖 Generated with Claude Code