Define Leadtype core and adapter boundary#51
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 (3)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/index.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (7)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR introduces the ChangesNext.js Adapter + Core/Adapter Boundary
Sequence Diagram(s)sequenceDiagram
participant Request
participant RouteHandler as createDocsRouteHandler
participant Agent as createAgentMarkdownResponse
participant Reader as readMarkdownFile
Request->>RouteHandler: incoming Request + headers
RouteHandler->>Agent: createAgentMarkdownResponse(requestHeaders, manifest)
Agent->>Reader: request markdown artifact (manifest target)
Reader-->>Agent: markdown string | null
Agent-->>RouteHandler: AgentMarkdownResponse | null
RouteHandler-->>Request: HTTP Response (markdown or 404)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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 |
…ine-leadtype-core-and-framework-adapter-bo # Conflicts: # packages/leadtype/package.json # packages/leadtype/rollup.config.ts # packages/leadtype/src/internal/package-surface.test.ts
![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: 0c70fde963
ℹ️ 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".
| requestOrigin: url.origin, | ||
| cacheControl: config.cacheControl, | ||
| }); | ||
| return response ?? new Response(null, { status: 404 }); |
There was a problem hiding this comment.
When this handler is mounted as documented on app/docs/[[...slug]]/route.ts, a normal browser GET with Accept: text/html makes createAgentMarkdownResponse return null, so this line returns a concrete 404. Next route handlers do not fall through to the page renderer after a Response, so every HTML docs page under that catch-all is served as 404 while only markdown/agent requests work.
Useful? React with 👍 / 👎.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
The documented Next.js recipe is broken: route.ts and page.tsx cannot coexist at the same App Router segment — next dev / next build errors with a route conflict. Both the docs and the JSDoc need a different layout (e.g. a sub-segment like raw/route.ts) before users can copy-paste this. A separate high-severity issue is that "use client" is silently stripped from the bundled dist/next/client.js.
TL;DR — Introduces the leadtype/next (server) and leadtype/next/client (React hook) adapter subpaths, formalises the core/adapter boundary in a new architecture doc, and enforces it with import-graph tests. The boundary doctrine and the surface tests are solid; the Next.js wiring shipped in the recipes does not match what Next.js allows.
Key changes
- Add
leadtype/nextserver adapter —createGenerateStaticParams,createLoadPageData, andcreateDocsRouteHandlerfactories with no React imports. - Add
leadtype/next/clientReact hook —useLeadtypeSearchplus a framework-freecreateSearchClient, both backed by a module-level artifact cache. - Document the core/adapter boundary — new
docs/reference/architecture.mdxdefining flat naming, no rendered DOM, and the optional-peer dependency rule; planned shapes for Nuxt/SvelteKit/Astro/TanStack Start named without exporting them yet. - Enforce the boundary in tests —
package-surface.test.tsnow scans everysrc/file for banned framework runtime imports and cross-adapter imports. - Wire the new subpaths through the toolchain —
rollup.config.tsentries,package.json#exports, optional peer + devreact/@types/react, README and recipe updates.
Summary ｜ 15 files ｜ 1 commit ｜ base: main ← KayleeWilliams/p0-define-leadtype-core-and-framework-adapter-bo
Boundary enforcement is the strongest part of the PR
Before: the core/adapter contract was implicit — fumadocs was the only adapter, and there was no scan preventing core code from importing React or other framework runtimes.
After:package-surface.test.tswalks every non-testsrc/file, flags banned specifiers in non-adapter dirs, and flags adapter-to-adapter relative imports.
The combination of the architecture doc, the optional-peer scheme, and the import-graph tests makes the contract enforceable as new adapters arrive. One small follow-up worth filing separately: the regex matches only static import/export statements, so a future dynamic import("react") in core would slip past the scan.
docs/reference/architecture.mdx · packages/leadtype/src/internal/package-surface.test.ts
Next.js recipe needs a different file layout
Before: users wired markdown content negotiation manually.
After: the recipe tells users to drop aroute.tsnext topage.tsxatapp/docs/[[...slug]]/, but Next.js disallows that exact layout, so the recipe fails atnext dev.
The Next.js Route Handlers docs state verbatim: "There cannot be a route.js file at the same route segment level as page.js" (source). The fix is to put the handler under a sub-segment (e.g. app/docs/[[...slug]]/raw/route.ts) or scope the markdown route to a different path entirely. The factory itself (createDocsRouteHandler in src/next/index.ts) is fine — only the documented placement and the JSDoc need to change.
docs/build/use-the-source-primitive.mdx · packages/leadtype/src/next/index.ts
Client bundle drops the "use client" directive
Before: N/A — new file.
After:src/next/client.tscarries"use client"at the top, butrollup.config.tsbundles in non-preserveModulesmode with norollup-plugin-preserve-directives, so the publisheddist/next/client.jswill not retain the directive.
Rollup and esbuild both strip top-level string directives in bundled output. Without "use client" on the published file, consumers who don't already nest the import inside their own "use client" boundary will hit the RSC "hooks in server components" error. The canonical fix is rollup-plugin-preserve-directives together with preserveModules: true, or splitting the client entry into its own preserve-modules pass.
packages/leadtype/rollup.config.ts · packages/leadtype/src/next/client.ts
Claude Opus ｜ 𝕏
| Add a sibling `route.ts` to serve raw markdown and content negotiation: | ||
|
|
||
| ```ts title="app/docs/[[...slug]]/route.ts" | ||
| import { createDocsRouteHandler } from "leadtype/next"; | ||
| import manifest from "@/generated/agent-readability.json" with { type: "json" }; | ||
|
|
||
| export const GET = createDocsRouteHandler({ | ||
| manifest: { ...manifest, version: 1 } as const, | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
This recipe will not run. Next.js's App Router rejects a route.ts at the same segment as page.tsx — next dev and next build error out with a route conflict (docs: "There cannot be a route.js file at the same route segment level as page.js", https://nextjs.org/docs/app/getting-started/route-handlers).
Move the handler to a sub-segment, e.g. app/docs/[[...slug]]/raw/route.ts, and update the JSDoc example in packages/leadtype/src/next/index.ts to match.
| * | ||
| * ```ts title="app/docs/[[...slug]]/route.ts" | ||
| * import { createDocsRouteHandler } from "leadtype/next"; | ||
| * import manifest from "@/generated/agent-readability.json" assert { type: "json" }; |
There was a problem hiding this comment.
Two issues in this JSDoc block:
- The example places
route.tsalongsidepage.tsxat the same segment — Next.js disallows this (see the same comment on the docs recipe). assert { type: "json" }is the deprecated import-attribute syntax. TS 5.3+ warns on it, and the mdx recipe added in this same PR uses the currentwith { type: "json" }form. Pick one — preferablywith, sincepackage.jsondeclarestypescript >=5.0and the docs already use it.
| "next/index": "src/next/index.ts", | ||
| "next/client": "src/next/client.ts", |
There was a problem hiding this comment.
src/next/client.ts starts with "use client", but this Rollup config bundles in non-preserveModules mode without rollup-plugin-preserve-directives, so both Rollup and esbuild will strip the directive from dist/next/client.js. Consumers who import useLeadtypeSearch from a server component will hit the RSC hooks-in-server-components error.
Add rollup-plugin-preserve-directives and set preserveModules: true (at minimum for the client entries), or split the client entry into its own build pass that preserves modules.
| /** | ||
| * Directory where the generate CLI wrote artifacts. Default: `./public` | ||
| * (resolved relative to `process.cwd()` at request time). | ||
| */ | ||
| publicDir?: string; |
There was a problem hiding this comment.
The JSDoc says publicDir is "resolved relative to process.cwd() at request time", but createDefaultMarkdownReader calls path.resolve(publicDir) once at handler-creation time (line 114) — cwd() is captured at module evaluation, not per request. Either defer the path.resolve into the returned async function or update the comment to say "at handler creation time".
There was a problem hiding this comment.
Note
Incremental review: commit 45462ce is a clean merge of main that brings the previously-shipped leadtype/i18n subpath into the exports list, the Rollup entries, and the surface test. The conflict resolution between the new ./i18n export and the new ./next / ./next/client exports is correct. The four findings from the prior review (Next.js route.ts/page.tsx conflict, stripped "use client" directive, assert vs with syntax, and the publicDir JSDoc) still stand.
Claude Opus ｜ 𝕏
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/internal/package-surface.test.ts`:
- Line 2: The test builds srcRoot using a URL.pathname which breaks on Windows
and for percent-encoded names; import and use fileURLToPath from "node:url" and
replace the URL.pathname usage when computing srcRoot (the variable referenced
in package-surface.test.ts) so you convert the URL to a proper platform path
(e.g., call fileURLToPath on the URL you currently use) instead of reading
.pathname or feeding raw URL path to glob(); update any references that
construct new URL(...) for the project root to use fileURLToPath(new URL(...,
import.meta.url)) accordingly.
- Around line 32-33: The current IMPORT_PATTERN only matches static
import/export and misses dynamic imports and CommonJS requires, so update
extractImportSpecifiers() to also detect dynamic import() calls and require(...)
calls: extend or add patterns alongside IMPORT_PATTERN to capture module strings
inside import("...") and require("...") (and their single-quoted variants), and
ensure these matches are fed into the same downstream boundary-check logic
(refer to IMPORT_PATTERN and the extractImportSpecifiers function name to locate
where to add the extra regexes/pattern handling).
🪄 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: c4f6b197-e000-40f1-9ba8-1d44c9dd2cf0
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.changeset/leadtype-next-adapter.mdREADME.mddocs/build/use-the-source-primitive.mdxdocs/docs.config.tsdocs/methodology.mdxdocs/reference/architecture.mdxpackages/leadtype/README.mdpackages/leadtype/package.jsonpackages/leadtype/rollup.config.tspackages/leadtype/src/internal/package-surface.test.tspackages/leadtype/src/next/client.test.tspackages/leadtype/src/next/client.tspackages/leadtype/src/next/index.tspackages/leadtype/src/next/next.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:
docs/docs.config.tspackages/leadtype/rollup.config.tspackages/leadtype/src/next/client.test.tspackages/leadtype/src/next/next.test.tspackages/leadtype/src/next/index.tspackages/leadtype/src/next/client.tspackages/leadtype/src/internal/package-surface.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:
docs/docs.config.tspackages/leadtype/rollup.config.tspackages/leadtype/src/next/client.test.tspackages/leadtype/src/next/next.test.tspackages/leadtype/src/next/index.tspackages/leadtype/src/next/client.tspackages/leadtype/src/internal/package-surface.test.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/next/client.test.tspackages/leadtype/src/next/next.test.tspackages/leadtype/src/internal/package-surface.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/next/index.ts
🪛 LanguageTool
packages/leadtype/README.md
[grammar] ~68-~68: Ensure spelling is correct
Context: ...pe/search/svelte are tracked under [#41](https://github.com/inthhq/leadtype/issues/41) / [#45](https://github.com/inthhq/leadtype/issues/45)). The leadtypebinary wrapsgenerate` a...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
docs/reference/architecture.mdx
[style] ~41-~41: Consider using “who” when you are referring to a person instead of an object.
Context: ...optional peer dependency*. A consumer that doesn't use fumadocs never installs fum...
(THAT_WHO)
[style] ~41-~41: Consider using “who” when you are referring to a person instead of an object.
Context: ...ever installs fumadocs-core; a consumer that doesn't use Next never installs React. ...
(THAT_WHO)
[grammar] ~56-~56: Ensure spelling is correct
Context: ...h artifacts. | These are future public surfaces, not hidden exports. Until they ship, u...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.22.1)
.changeset/leadtype-next-adapter.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (34)
packages/leadtype/package.json (1)
40-47: LGTM!Also applies to: 137-137, 141-141, 156-156, 181-183
packages/leadtype/rollup.config.ts (1)
11-12: LGTM!.changeset/leadtype-next-adapter.md (1)
1-11: LGTM!docs/docs.config.ts (1)
23-23: LGTM!README.md (1)
26-26: LGTM!Also applies to: 66-66
packages/leadtype/README.md (1)
64-68: LGTM!docs/build/use-the-source-primitive.mdx (1)
43-44: LGTM!Also applies to: 62-66, 69-71, 75-75, 86-86, 88-99, 209-214, 215-233, 247-248, 260-260, 274-274
packages/leadtype/src/next/index.ts (7)
36-49: LGTM!
51-76: LGTM!
84-91: LGTM!
97-101: LGTM!
103-109: LGTM!
111-134: LGTM!
145-168: LGTM!packages/leadtype/src/next/next.test.ts (4)
12-38: LGTM!
40-56: LGTM!
58-124: LGTM!
126-200: LGTM!packages/leadtype/src/next/client.ts (9)
1-40: LGTM!
42-69: LGTM!
71-79: LGTM!
81-110: LGTM!
117-149: LGTM!
151-169: LGTM!
178-218: LGTM!
220-261: LGTM!
263-264: LGTM!packages/leadtype/src/next/client.test.ts (5)
9-39: LGTM!
41-53: LGTM!
55-66: LGTM!
68-141: LGTM!
143-165: LGTM!docs/reference/architecture.mdx (1)
1-93: LGTM!docs/methodology.mdx (1)
27-27: LGTM!Also applies to: 35-36
|
API Error: Repeated 529 Overloaded errors · check status.claude.com
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd9b314f96
ℹ️ 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".
| Add a sibling `route.ts` to serve raw markdown and content negotiation: | ||
|
|
||
| ```ts title="app/docs/[[...slug]]/route.ts" |
There was a problem hiding this comment.
In the documented Next App Router setup, this route.ts is added in the same app/docs/[[...slug]] segment as the preceding page.tsx, but Next does not allow a route handler and a page at the same route segment level. Consumers following the recommended adapter path will hit a routing/build error before markdown negotiation can work; the handler needs to be mounted at a different segment or implemented via middleware/proxy instead.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,317 @@ | |||
| "use client"; | |||
|
|
|||
| import { useCallback, useEffect, useRef, useState } from "react"; | |||
There was a problem hiding this comment.
When consumers use the documented createSearchClient in a worker, plain script, or non-React web component, this top-level React import is resolved before they ever access the hook. Because React is only an optional peer, installs that only want the “framework-free” factory can fail with a missing react module; the vanilla fetch/search client needs to live in a subpath/module that does not import React.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/leadtype/src/next/index.ts (1)
145-148:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winJSDoc example still suggests invalid Next.js route placement.
The remark says to place the handler "next to the matching
page.tsx," but Next.js App Router does not allow bothroute.tsandpage.tsxat the same segment—they are mutually exclusive. Either remove the reference topage.tsxor clarify that the route handler should be at a path that does not have apage.tsx.📝 Suggested JSDoc fix
* `@remarks` - * Place the generated handler at `app/docs/[[...slug]]/route.ts` next to the - * matching `page.tsx`. It returns markdown when the request is agent-readable + * Place the generated handler at `app/docs/[[...slug]]/route.ts`. It returns markdown when the request is agent-readable * and a 404 response otherwise.🤖 Prompt for 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. In `@packages/leadtype/src/next/index.ts` around lines 145 - 148, Update the JSDoc remark that currently instructs to "Place the generated handler at `app/docs/[[...slug]]/route.ts` next to the matching `page.tsx`": replace the phrase about being "next to the matching `page.tsx`" with a clear statement that the route handler must be placed in a segment that does not have a `page.tsx` (or simply remove the `page.tsx` reference), so the remark unambiguously indicates the handler should be in a path without a `page.tsx` (edit the remark block above the generated handler documentation in the JSDoc).
🤖 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/next/index.ts`:
- Around line 169-171: The header-copy loop in index.ts that iterates over
request.headers and assigns to the headers object can be simplified; replace the
manual forEach loop with a single assignment using
Object.fromEntries(request.headers) (or
Object.fromEntries(request.headers.entries())) to produce the plain object,
i.e., set headers = Object.fromEntries(request.headers) where headers and
request.headers are referenced.
---
Duplicate comments:
In `@packages/leadtype/src/next/index.ts`:
- Around line 145-148: Update the JSDoc remark that currently instructs to
"Place the generated handler at `app/docs/[[...slug]]/route.ts` next to the
matching `page.tsx`": replace the phrase about being "next to the matching
`page.tsx`" with a clear statement that the route handler must be placed in a
segment that does not have a `page.tsx` (or simply remove the `page.tsx`
reference), so the remark unambiguously indicates the handler should be in a
path without a `page.tsx` (edit the remark block above the generated handler
documentation in the JSDoc).
🪄 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: 377b5403-3d27-4e86-9255-a507f2205aa7
📒 Files selected for processing (2)
packages/leadtype/src/next/client.tspackages/leadtype/src/next/index.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/next/index.tspackages/leadtype/src/next/client.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/next/index.tspackages/leadtype/src/next/client.ts
**/index.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid barrel files (index files that re-export everything)
Files:
packages/leadtype/src/next/index.ts
🔇 Additional comments (2)
packages/leadtype/src/next/index.ts (1)
1-14: LGTM!Also applies to: 16-87, 89-138, 160-183
packages/leadtype/src/next/client.ts (1)
1-317: LGTM!
There was a problem hiding this comment.
No new issues. Reviewed the following changes:
- Tightened the
package-surface.test.tsboundary scan to also flag dynamicimport(...)andrequire(...)specifiers, and switchedsrcRoottofileURLToPathso the scan resolves correctly on Windows. - Reworked
leadtype/nextandleadtype/next/clientJSDoc into per-symbol blocks with@param,@defaultValue,@remarks, and@exampletags. - Corrected the
createDocsRouteHandlerJSDoc to require a route segment without a siblingpage.tsx, swapped the import-attributes example fromasserttowith, and dropped the inaccurate "resolved at request time" claim onpublicDir. - Replaced the headers
forEachaccumulator in the route handler withObject.fromEntries(request.headers).
Claude Opus ｜ 𝕏
KayleeWilliams
deleted the
KayleeWilliams/p0-define-leadtype-core-and-framework-adapter-bo
branch
1 participant
Summary
leadtype/nextandleadtype/next/client.mainand preserves the newleadtype/i18npackage surface alongside the Next adapter exports.Closes #40.
Validation
bun test packages/leadtype/src/internal/package-surface.test.tsbun test packages/leadtype/src/next/next.test.ts packages/leadtype/src/next/client.test.tsbun test packages/leadtype/src/internal/package-surface.test.ts packages/leadtype/src/i18n/i18n.test.ts packages/leadtype/src/next/next.test.ts packages/leadtype/src/next/client.test.tsbun run --filter leadtype check-typesbun x ultracite checkbun run --filter leadtype build