Fix type table source root resolution#37
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 (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR makes type-table extraction file-aware and configurable: base-paths default to the docs/source root (with override via typeTableBasePath) and extraction failures can warn or fail (typeTableStrict). Options are exposed via createMdxSourcePlugins, wired through createDocsSource and the CLI, and covered by tests and docs. ChangesType-table resolution and failure handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The empty-properties check in remarkResolveTypeTableJsx flags successfully-extracted empty interfaces as failures, and the lastIndexOf("docs") heuristic has surprising fallbacks. Docs and the in-tree apps/example/vite.config.ts still teach the bare mdxSourcePlugins constant whose default semantics have silently changed — neither is updated in this PR.
TL;DR — Fixes type-table path= resolution so projects with non-docs/ source layouts (e.g. .c15t, .leadtype) actually find their referenced TypeScript files, and surfaces extraction failures via a visible warning (default) or a hard error (typeTableStrict).
Key changes
- Default base path moves to the source root —
<ExtractedTypeTable>and<AutoTypeTable>resolve relative to the parent of the docs dir (or the per-MDX-file vfile path), notprocess.cwd()/docs. - New
createMdxSourcePlugins(opts)factory — acceptstypeTableBasePath,typeTableStrict,typeTableWarnOnFailure; baremdxSourcePluginsconstant kept for back-compat but now defaults to nobasePath. - Config plumbing —
DocsConfigandCreateDocsSourceConfiggaintypeTableBasePath/typeTableStrict; CLIgenerateresolvestypeTableBasePathrelative to--src. - Visible failure surfacing — extraction failures emit a
**Warning:**paragraph (source pipeline) or italic failure note (CLI markdown) by default, and throw understrict;convertAllMdxgainsfailOnError. - Per-file basePath — processor signature in
generic-processor.tsnow threadsVFilethrough so resolution can use the source MDX's vfile path.
Summary ｜ 14 files ｜ 1 commit ｜ base: main ← type-table-source-root
Empty interfaces are misclassified as failures
Before: any extraction returning
nulltriggered the empty-properties fallback path.
After: any extraction whose result has zero keys — including a legitimateexport interface Marker {}— triggers the warning paragraph and, understrict, throws.
The new gate at type-table-jsx.remark.ts:169 is !(extracted && Object.keys(extracted).length > 0). extractTypeFromFile returns null only on real extraction failures; a successfully resolved but empty interface returns {}, which this gate treats as a failure. Note that the analogous gate in type-table.remark.ts:836 correctly uses if (extractedType), so this asymmetry is local to the JSX-resolution path.
packages/leadtype/src/remark/plugins/type-table-jsx.remark.ts
resolveDefaultTypeTableBasePath heuristic edge cases
Before: default basePath was always
process.cwd()/docs, wrong for non-docs/layouts.
After: default is derived from the vfile path viasegments.lastIndexOf("docs"), with surprising behavior in several plausible layouts.
The heuristic fires for the bare mdxSourcePlugins consumer path. Notable edge cases: nested …/docs/…/docs/x.mdx picks the deepest docs (likely not what the author meant); a file whose first segment is docs (relative path) falls through to process.cwd() because docsIndex > 0; on Windows the normalized split returns a mixed-separator string. createDocsSource and the CLI bypass this via explicit typeTableBasePath, so the heuristic primarily affects bundler consumers using the bare constant.
packages/leadtype/src/remark/plugins/type-table.remark.ts
Stale docs and in-tree consumer
Before: docs and
apps/example/vite.config.tsconsumedmdxSourcePlugins(constant), backed by the old default.
After: the constant still works but its default is now derived from vfile path; the newcreateMdxSourcePluginsfactory and the two new options are nowhere in user-facing docs.
Findings about files outside the diff, ordered by severity:
- Silent behavior change for an in-tree consumer:
apps/example/vite.config.tsstill imports the baremdxSourcePlugins. With the new heuristic onapps/example/content/docs/guides/…, the auto-resolved base path becomesapps/example/content, not the repo root the existing type fixtures live under. Confirm whether this app needs the samecreateMdxSourcePlugins({ typeTableBasePath })switch thatapps/fumadocs-example/next.config.mjsgot. - Reference docs missing the new options:
docs/reference/source.mdx'screateDocsSourceoptions table omitstypeTableBasePathandtypeTableStrict;docs/reference/mdx.mdxanddocs/reference/remark.mdxstill document the bare constant withprocess.cwd()examples;docs/build/use-the-source-primitive.mdx,docs/build/integrate-with-fumadocs.mdx,docs/build/build-a-docs-site.mdx, anddocs/quickstart.mdxteach the bare import. Per repo learnings,packages/leadtype/README.mdships in the npm tarball — its references to the old setup are publicly visible after release. - Missing migration note: users whose types genuinely live under
docs/(and who therefore relied oncwd()/docs) have no in-repo guidance to opt back in viacreateMdxSourcePlugins({ typeTableBasePath: path.resolve(process.cwd(), "docs") }). - Patch changeset for an additive export + default-behavior change on a v1 surface — defensible only on the "the old default was broken for the target use case" theory, but worth a deliberate call rather than the default.
- Stale doc-comments:
packages/leadtype/src/mdx/index.tsheader andpackages/leadtype/src/source/index.tsJSDoc forremarkPlugins?still referencemdxSourcePluginsas the documented default;apps/fumadocs-example/lib/source.ts:19comment is now factually wrong becausenext.config.mjsswitched to the factory.
Test coverage gaps
Before: the CLI and source pipelines emitted divergent failure markdown (italic vs
**Warning:**).
After: strict-mode CLI behavior is covered; non-strict CLI markdown emission, theprocess.cwd()fallback branch inresolveDefaultTypeTableBasePath, and the empty-interface edge case are not.
Optional additions: a CLI test asserting the italic *Could not extract …* paragraph appears in the generated markdown when typeTableStrict is unset; a direct unit test of resolveDefaultTypeTableBasePath for undefined and no-docs-segment inputs; an assertion that a legitimate export interface Empty {} does not trigger the warning/strict path.
packages/leadtype/src/source/source.test.ts · packages/leadtype/src/cli.test.ts · packages/leadtype/src/remark/remark-output.test.ts
Claude Opus ｜ 𝕏
| const extracted = extractTypeFromFile(path, name, overrideBasePath); | ||
| const typeTableNode = buildTypeTableNode({ | ||
| properties: extracted ?? {}, | ||
| title, | ||
| description, | ||
| name, | ||
| path, | ||
| }); | ||
| if (!(extracted && Object.keys(extracted).length > 0)) { |
There was a problem hiding this comment.
An empty interface like export interface Marker {} extracts successfully to {}, but this gate Object.keys(extracted).length > 0 classifies it as failure — emitting a Warning: paragraph in non-strict mode and throwing under strict. Distinguish extracted == null (real failure) from Object.keys(extracted).length === 0 (valid empty type). The sibling check in type-table.remark.ts:836 uses if (extractedType) and gets this right.
| const extracted = extractTypeFromFile(path, name, overrideBasePath); | |
| const typeTableNode = buildTypeTableNode({ | |
| properties: extracted ?? {}, | |
| title, | |
| description, | |
| name, | |
| path, | |
| }); | |
| if (!(extracted && Object.keys(extracted).length > 0)) { | |
| // implement one runtime component. If extraction fails, emit a | |
| // visible warning before the placeholder table unless strict mode | |
| // has been enabled. | |
| const extracted = extractTypeFromFile(path, name, overrideBasePath); | |
| const typeTableNode = buildTypeTableNode({ | |
| properties: extracted ?? {}, | |
| title, | |
| description, | |
| name, | |
| path, | |
| }); | |
| if (!extracted) { |
| } | ||
|
|
||
| return process.cwd(); | ||
| } |
There was a problem hiding this comment.
lastIndexOf("docs") produces surprising results for layouts the heuristic plausibly meets: nested …/docs/…/docs/x.mdx picks the deepest docs (likely not the intended source root); a relative path whose first segment is docs (e.g. docs/x.mdx) falls through to process.cwd() because docsIndex > 0; on Windows the returned string mixes separators. Worth either narrowing this to fire only when the parent of the MDX file is docs (use segments[segments.length - 2]), or pushing harder in the docs toward explicit typeTableBasePath so the heuristic only ever runs as a last-resort fallback.
| const remarkPlugins = config.remarkPlugins ?? mdxSourcePlugins; | ||
| const typeTableBasePath = path.resolve( | ||
| config.typeTableBasePath ?? path.dirname(contentDir) | ||
| ); |
There was a problem hiding this comment.
When contentDir is at the filesystem root (path.dirname("/") === "/") or is a single-segment relative path (path.dirname("./docs") === "."), this resolves to / or process.cwd(), neither of which is a useful type-resolution root. Probably fine for any realistic project layout, but worth a one-line guard that falls back to contentDir itself when path.dirname(contentDir) === contentDir or === ".".
| ]; | ||
| } | ||
|
|
||
| export const mdxSourcePlugins: PluggableList = createMdxSourcePlugins(); |
There was a problem hiding this comment.
Retaining mdxSourcePlugins as createMdxSourcePlugins() (no options) means existing consumers of the bare constant silently pick up the new vfile-derived default. That's the intended fix for the documented use case, but bundler consumers whose source MDX has docs in an unintended location (e.g. inside node_modules/foo/docs/…) will get a different basePath than they did before with no migration path other than swapping to the factory. Consider calling this out in the changeset and/or in the JSDoc above the constant so the silent behavior change is at least discoverable.
There was a problem hiding this comment.
Note
Prior feedback addressed cleanly. One cosmetic stale string remains in apps/fumadocs-example/app/page.tsx:11 (outside this diff): the JSX still reads "resolved at build time by mdxSourcePlugins" while the example's next.config.mjs now wires createMdxSourcePlugins({ typeTableBasePath }). Worth a follow-up sweep but not blocking.
Reviewed the following changes:
- Switched
resolveDefaultTypeTableBasePathtoindexOfso the firstdocssegment wins, wrapped the result innormalize, and added a direct unit test covering nested-docs/.../docs, relative-docs, and no-docsinputs. - Simplified the JSX-path empty-extraction gate to
!extracted, so a legitimateexport interface Marker {}no longer trips the warning/strict branch; covered by a newsource.test.tscase. - Hardened
createDocsSource's defaulttypeTableBasePathagainst the root-contentDiredge by falling back tocontentDirwhenpath.dirname(contentDir) === contentDir. - Updated
apps/example/vite.config.tsto usecreateMdxSourcePlugins({ typeTableBasePath })(resolving to the monorepo root), matching the priorapps/fumadocs-example/next.config.mjsswitch. - Refreshed the docs surface across
docs/quickstart,docs/build/*,docs/reference/{mdx,source,remark},packages/leadtype/README.md, and internal JSDocs to teachcreateMdxSourcePlugins({ typeTableBasePath }), document the newtypeTableBasePath/typeTableStrictoptions oncreateDocsSource, and include migration guidance for projects whose types live underdocs/.
Claude Opus ｜ 𝕏
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 `@apps/example/vite.config.ts`:
- Line 14: The variable typeTableBasePath is currently computed via
resolve(process.cwd(), "..", "..") which varies based on where Vite is invoked;
change it to compute the path relative to this config file using
import.meta.url: call fileURLToPath(import.meta.url) then dirname(...) and
resolve up two levels to produce a stable file-relative base path; update the
identifier typeTableBasePath accordingly so all downstream code (type-table
extraction) uses the anchored path.
🪄 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: cb6e0cdb-cfb8-42be-87c9-ed2264e8bb52
📒 Files selected for processing (18)
.changeset/type-table-source-root.mdapps/example/vite.config.tsapps/fumadocs-example/lib/source.tsdocs/build/build-a-docs-site.mdxdocs/build/integrate-with-fumadocs.mdxdocs/build/use-the-source-primitive.mdxdocs/quickstart.mdxdocs/reference/mdx.mdxdocs/reference/remark.mdxdocs/reference/source.mdxpackages/leadtype/README.mdpackages/leadtype/src/mdx/index.tspackages/leadtype/src/mdx/source-preset.tspackages/leadtype/src/remark/plugins/type-table-jsx.remark.tspackages/leadtype/src/remark/plugins/type-table.remark.tspackages/leadtype/src/remark/remark-output.test.tspackages/leadtype/src/source/index.tspackages/leadtype/src/source/source.test.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:
apps/fumadocs-example/lib/source.tspackages/leadtype/src/mdx/index.tspackages/leadtype/src/source/index.tspackages/leadtype/src/mdx/source-preset.tsapps/example/vite.config.tspackages/leadtype/src/remark/remark-output.test.tspackages/leadtype/src/remark/plugins/type-table-jsx.remark.tspackages/leadtype/src/remark/plugins/type-table.remark.tspackages/leadtype/src/source/source.test.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:
apps/fumadocs-example/lib/source.tspackages/leadtype/src/mdx/index.tspackages/leadtype/src/source/index.tspackages/leadtype/src/mdx/source-preset.tsapps/example/vite.config.tspackages/leadtype/src/remark/remark-output.test.tspackages/leadtype/src/remark/plugins/type-table-jsx.remark.tspackages/leadtype/src/remark/plugins/type-table.remark.tspackages/leadtype/src/source/source.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.tspackages/leadtype/src/source/index.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/remark/remark-output.test.tspackages/leadtype/src/source/source.test.ts
🪛 markdownlint-cli2 (0.22.1)
.changeset/type-table-source-root.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (18)
packages/leadtype/src/remark/plugins/type-table.remark.ts (1)
3-3: LGTM!Also applies to: 8-9, 157-160, 172-172, 179-211, 213-247, 837-843, 857-857, 979-990
packages/leadtype/src/remark/plugins/type-table-jsx.remark.ts (1)
14-22: LGTM!Also applies to: 27-30, 90-104, 108-111, 115-124, 158-160, 162-185, 189-189
packages/leadtype/src/remark/remark-output.test.ts (1)
11-11: LGTM!Also applies to: 81-81, 87-87, 117-117, 143-153
packages/leadtype/src/mdx/source-preset.ts (1)
24-31: LGTM!Also applies to: 39-55, 57-63
packages/leadtype/src/mdx/index.ts (1)
10-13: LGTM!Also applies to: 42-46
docs/build/build-a-docs-site.mdx (1)
25-25: LGTM!apps/fumadocs-example/lib/source.ts (1)
19-19: LGTM!packages/leadtype/README.md (1)
54-54: LGTM!docs/reference/remark.mdx (1)
134-147: LGTM!apps/example/vite.config.ts (1)
1-6: LGTM!Also applies to: 43-50
packages/leadtype/src/source/index.ts (1)
12-12: LGTM!Also applies to: 41-42, 100-112, 223-234
packages/leadtype/src/source/source.test.ts (1)
14-25: LGTM!Also applies to: 118-251
.changeset/type-table-source-root.md (1)
1-9: LGTM!docs/quickstart.mdx (1)
56-57: LGTM!Also applies to: 71-72, 81-82, 86-87
docs/build/integrate-with-fumadocs.mdx (1)
31-39: LGTM!Also applies to: 79-87, 93-93, 142-142
docs/build/use-the-source-primitive.mdx (1)
27-27: LGTM!Also applies to: 39-39, 45-53, 90-94, 105-115, 197-227, 234-235, 328-330, 335-335
docs/reference/source.mdx (1)
38-40: LGTM!Also applies to: 102-102, 116-124, 144-148, 159-160, 164-174, 190-195, 203-209, 217-222, 233-233
docs/reference/mdx.mdx (1)
14-14: LGTM!Also applies to: 23-27, 30-32, 37-38, 228-228
There was a problem hiding this comment.
No new issues. Reviewed the following changes:
- Anchored
apps/example/vite.config.ts'stypeTableBasePathto the config file's own directory viadirname(fileURLToPath(import.meta.url))instead ofprocess.cwd(), so resolution is stable regardless of where Vite is invoked from.
Claude Opus ｜ 𝕏
1 participant
Summary
Why
Projects can keep Leadtype-authored docs under source roots such as .c15t or .leadtype while referenced TypeScript packages live beside docs, not inside docs. Resolving type-table paths from docs caused valid references like ./packages/react/... to miss the real source file and produce empty props tables.
Validation