feat(ui-core): extract DOM-free components from @kopai/ui#123
Merged
Conversation
- allows reuse in ReactNative
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new DOM-free package Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Provider as KopaiSDKProvider
participant Renderer
participant Hook as useKopaiData
participant ReactQuery as QueryClient
participant Client as KopaiClient
App->>Provider: Wrap app with KopaiSDKProvider(client)
App->>Renderer: Render tree (passes registry)
Renderer->>Hook: request data for element (dataSource)
Hook->>ReactQuery: useQuery(queryKey, queryFn)
ReactQuery->>Client: invoke SDK method (with AbortSignal)
Client-->>ReactQuery: return response / throw error
ReactQuery-->>Hook: resolve data / error
Hook-->>Renderer: provide response, loading, error, refetch, updateParams
Renderer-->>App: render component with data props
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
packages/ui-core/src/lib/log-buffer.ts (1)
20-22: GuardmaxSizeat construction time.If
maxSize <= 0(or non-integer), buffer behavior becomes unintuitive (immediate/full trimming). A constructor guard will fail fast and avoid hidden misconfiguration.🔧 Add validation for `maxSize`
constructor(maxSize = 1000) { + if (!Number.isInteger(maxSize) || maxSize <= 0) { + throw new Error("maxSize must be a positive integer"); + } this.maxSize = maxSize; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-core/src/lib/log-buffer.ts` around lines 20 - 22, Add a construction-time guard in the LogBuffer constructor to ensure the incoming maxSize is a positive integer; replace the direct assignment this.maxSize = maxSize with a validation that uses Number.isInteger(maxSize) && maxSize > 0 and throw a clear RangeError (or TypeError) if it fails so misconfiguration fails fast while keeping the default 1000 behavior when no arg is provided.packages/ui-core/src/providers/kopai-provider.tsx (1)
25-33: SharedqueryClientsingleton may cause issues in SSR or multiple-provider scenarios.The exported
queryClientinstance works well for client-side SPAs and testing (as seen inuse-live-logs.test.tsusingqueryClient.clear()). However, this singleton pattern can cause cache sharing issues if:
- Multiple
KopaiSDKProviderinstances are mounted simultaneously- The package is used in SSR where request isolation is required
If SSR support or multiple isolated providers are needed in the future, consider allowing an optional
queryClientprop to override the default.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-core/src/providers/kopai-provider.tsx` around lines 25 - 33, The current exported singleton queryClient can cause cross-request or multi-provider cache sharing; change KopaiSDKProvider to accept an optional queryClient prop and stop relying on the module-level exported queryClient as the default — create a new QueryClient inside KopaiSDKProvider when no prop is passed (use the same defaultOptions), remove or deprecate the global export for runtime consumers (keep for tests only if necessary), and ensure all internal uses reference the provider's prop (or internal instance) rather than the module-level queryClient; target symbols: queryClient and KopaiSDKProvider.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui-core/package.json`:
- Around line 16-26: The package's conditional "exports" currently only points
"types" to "./dist/index.d.mts", which leaves CJS consumers without type
declarations; update the "exports" mapping for "." to provide conditional types
for both ESM and CJS (e.g., under "import" set "types" -> "./dist/index.d.mts"
and "default" -> "./dist/index.mjs", and under "require" set "types" ->
"./dist/index.d.cts" and "default" -> "./dist/index.cjs"), and then either align
or remove the top-level "types" field so it doesn't conflict with the
conditional exports (ensure references to dist/index.d.mts and dist/index.d.cts
match the generated declaration files).
In `@packages/ui-core/src/hooks/use-live-logs.ts`:
- Line 35: The hook currently initializes and resumes live mode regardless of
the passed-in enabled flag (isLive is set true by default and setLive(true)
always calls refetch()), so gate live behavior behind enabled: initialize isLive
from enabled (or set to false if enabled is false), make setLive respect enabled
(only allow setting true and calling refetch() when enabled is true), and ensure
refetchInterval and the manual refetch() invocation are conditional on enabled;
update references in use-live-logs.ts for isLive, setIsLiveState/setLive,
refetch(), and refetchInterval so no network activity or “live” state is exposed
when enabled is false.
In `@packages/ui-core/src/lib/component-catalog.ts`:
- Around line 68-77: The public schema currently allows props: z.unknown(), but
code in createCatalog (when building uiTreeSchema) and
generatePromptInstructions (calling z.toJSONSchema()) expects component.props to
be a Zod schema; change the componentDefinitionSchema and any other
component-like Zod objects (the other occurrence around the 132-156 block) to
use props: z.ZodTypeAny(), update types to reflect ZodTypeAny for
component.props, and ensure callers (createCatalog, uiTreeSchema construction,
generatePromptInstructions) treat component.props as a Zod schema rather than
plain objects so runtime z.toJSONSchema() calls are safe.
In `@packages/ui-core/src/lib/generate-prompt-instructions.ts`:
- Around line 154-156: The code is overwriting any existing JSON Schema
definitions by replacing treeJsonSchema.$defs; instead, preserve existing $defs
and inject DataSource into the map. Update the logic around
replaceDataSourceWithRef and the assignment to treeJsonSchema.$defs so you merge
into the existing treeJsonSchema.$defs (or create it if missing) and set only
the DataSource key to dataSourceJsonSchema, leaving other $defs intact to avoid
dangling $ref references.
In `@packages/ui-core/src/lib/log-buffer.ts`:
- Around line 5-13: The dedupe key in function logKey is using a 32-bit rolling
int hash of OtelLogsRow.Body which can collide and silently drop distinct logs;
update logKey to compute a cryptographic hash (e.g., SHA-256) of row.Body and
use its hex (or base64) output instead of the current 32-bit int so the returned
key (`${row.Timestamp}-${row.ServiceName ?? ""}-${hash}`) reliably distinguishes
different bodies; locate logKey and replace the bitwise hash logic with a secure
hash computation (using Node's crypto.createHash('sha256') or Web Crypto) while
keeping Timestamp and ServiceName in the key.
In `@packages/ui-core/src/lib/observability-catalog.ts`:
- Around line 8-15: The Zod schemas for component props (e.g., the Card.props
z.object in observability-catalog and the similar schemas for Grid, Stack,
Heading, Text, Badge, and metric widgets) use .nullable(), which still requires
the key to be present and causes validation failures when props are omitted;
update these prop fields to allow omission by using .nullish() or
.optional().nullable() for each prop type inside the props z.object used by
createCatalog() so that { props: {} } and the example tree validate correctly.
In `@packages/ui-core/src/lib/renderer.test.tsx`:
- Around line 128-134: The test uses createRendererFromCatalog incorrectly: it
returns an object { Renderer, registry } rather than a component, and the second
argument is not the component registry. Change each instantiation (e.g. the
TestRenderer/LocalRenderer/IntegrationRenderer assignments) to destructure the
result: const { Renderer, registry } = createRendererFromCatalog(_testCatalog,
/* options if any */); then update places that call createElement(TestRenderer,
{ tree }) to use createElement(Renderer, { tree, registry }) or otherwise supply
the returned registry to the Renderer invocation. Apply the same fix to the
other occurrences referenced (lines near 260-266, 429-435, 561-564, 655-658,
671-673, 695-699, 726-731) so tests use the Renderer component and registry
returned from createRendererFromCatalog instead of treating the call as a
component or registry argument.
In `@packages/ui-core/src/lib/renderer.tsx`:
- Around line 225-239: The hook ordering is unstable because updateParams
(created via useCallback) is declared after a conditional early return that runs
when methodIsAccepted is false; move the useCallback declaration for
updateParams (and any other hooks) above the conditional that returns null so
hooks run in the same order every render, keeping the same dependencies (e.g.,
setParamsOverride) and preserving the function name updateParams; ensure the
conditional that logs the error (using methodIsAccepted, element.dataSource,
acceptsDataFrom) still returns null when appropriate after the move.
- Around line 26-30: The UITree and UIElement types are currently unbound to a
specific catalog; change them to generics so renderer instances are
catalog-safe: introduce a generic constraint like CatalogLike and redefine
UITree as UITree<C extends CatalogLike> = z.infer<C["uiTreeSchema"]> and
UIElement as UIElement<C extends CatalogLike> = UITree<C>["elements"][string];
then update all renderer-related signatures (e.g., the Renderer type, render
function parameters, and any functions referencing UITree or UIElement) to
accept UITree<C> or UIElement<C> and ensure createCatalog usages and type
parameters propagate the specific catalog type C through the renderer factory.
In `@packages/ui-core/tsdown.config.ts`:
- Line 8: The tsdown.config.ts marks "zod" as external, so update
packages/ui-core/package.json to list "zod" in peerDependencies (e.g., a semver
range like "^x.y.z") so consumers know they must provide it; also remove it from
runtime dependencies if present (or keep it only as a devDependency for local
builds/tests) and ensure the version in peerDependencies matches the version
used in devDependencies/tests; reference the package.json’s peerDependencies
section and the tsdown.config.ts external array to make the change.
---
Nitpick comments:
In `@packages/ui-core/src/lib/log-buffer.ts`:
- Around line 20-22: Add a construction-time guard in the LogBuffer constructor
to ensure the incoming maxSize is a positive integer; replace the direct
assignment this.maxSize = maxSize with a validation that uses
Number.isInteger(maxSize) && maxSize > 0 and throw a clear RangeError (or
TypeError) if it fails so misconfiguration fails fast while keeping the default
1000 behavior when no arg is provided.
In `@packages/ui-core/src/providers/kopai-provider.tsx`:
- Around line 25-33: The current exported singleton queryClient can cause
cross-request or multi-provider cache sharing; change KopaiSDKProvider to accept
an optional queryClient prop and stop relying on the module-level exported
queryClient as the default — create a new QueryClient inside KopaiSDKProvider
when no prop is passed (use the same defaultOptions), remove or deprecate the
global export for runtime consumers (keep for tests only if necessary), and
ensure all internal uses reference the provider's prop (or internal instance)
rather than the module-level queryClient; target symbols: queryClient and
KopaiSDKProvider.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 33a95be8-4109-4b93-8c01-30e674b4ad55
⛔ Files ignored due to path filters (2)
packages/ui-core/src/lib/__snapshots__/generate-prompt-instructions.test.ts.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
.changeset/ui-core-initial.mdREADME.mdexamples/ui-react-app/README.mdexamples/ui-react-app/package.jsonexamples/ui-react-app/src/custom-observability-catalog.tsxexamples/ui-react-app/src/dynamic-kopai-data.tsxexamples/ui-react-app/src/static-data.tsxpackages/ui-core/package.jsonpackages/ui-core/src/hooks/use-kopai-data.test.tspackages/ui-core/src/hooks/use-kopai-data.tspackages/ui-core/src/hooks/use-live-logs.test.tspackages/ui-core/src/hooks/use-live-logs.tspackages/ui-core/src/index.tspackages/ui-core/src/lib/component-catalog.test.tspackages/ui-core/src/lib/component-catalog.tspackages/ui-core/src/lib/generate-prompt-instructions.test.tspackages/ui-core/src/lib/generate-prompt-instructions.tspackages/ui-core/src/lib/log-buffer.test.tspackages/ui-core/src/lib/log-buffer.tspackages/ui-core/src/lib/observability-catalog.tspackages/ui-core/src/lib/renderer.test.tsxpackages/ui-core/src/lib/renderer.tsxpackages/ui-core/src/providers/kopai-provider.tsxpackages/ui-core/tsconfig.jsonpackages/ui-core/tsdown.config.ts
| const client = useKopaiSDK(); | ||
| const bufferRef = useRef(new LogBuffer(maxLogs)); | ||
| const [version, setVersion] = useState(0); | ||
| const [isLive, setIsLiveState] = useState(true); |
There was a problem hiding this comment.
enabled is bypassed when live mode is resumed.
The hook starts with isLive === true regardless of enabled, and setLive(true) always calls refetch(). A consumer that passes enabled: false can still trigger network traffic and sees a misleading live state. Gate the live state, refetchInterval, and the manual refetch() path behind enabled.
Also applies to: 86-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-core/src/hooks/use-live-logs.ts` at line 35, The hook currently
initializes and resumes live mode regardless of the passed-in enabled flag
(isLive is set true by default and setLive(true) always calls refetch()), so
gate live behavior behind enabled: initialize isLive from enabled (or set to
false if enabled is false), make setLive respect enabled (only allow setting
true and calling refetch() when enabled is true), and ensure refetchInterval and
the manual refetch() invocation are conditional on enabled; update references in
use-live-logs.ts for isLive, setIsLiveState/setLive, refetch(), and
refetchInterval so no network activity or “live” state is exposed when enabled
is false.
Comment on lines
+68
to
+77
| export const componentDefinitionSchema = z | ||
| .object({ | ||
| hasChildren: z.boolean(), | ||
| description: z | ||
| .string() | ||
| .describe( | ||
| "Component description to be displayed by the prompt generator" | ||
| ), | ||
| props: z.unknown(), | ||
| acceptsDataFrom: z.array(dataSourceMethodSchema).readonly().optional(), |
There was a problem hiding this comment.
props should be constrained to a Zod schema.
This API currently accepts props: unknown, but createCatalog() immediately treats component.props as a Zod schema when building uiTreeSchema, and generatePromptInstructions() does the same for z.toJSONSchema(). A plain object can type-check here and then blow up at runtime. Tighten the public type/signature to z.ZodTypeAny.
Also applies to: 132-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-core/src/lib/component-catalog.ts` around lines 68 - 77, The
public schema currently allows props: z.unknown(), but code in createCatalog
(when building uiTreeSchema) and generatePromptInstructions (calling
z.toJSONSchema()) expects component.props to be a Zod schema; change the
componentDefinitionSchema and any other component-like Zod objects (the other
occurrence around the 132-156 block) to use props: z.ZodTypeAny(), update types
to reflect ZodTypeAny for component.props, and ensure callers (createCatalog,
uiTreeSchema construction, generatePromptInstructions) treat component.props as
a Zod schema rather than plain objects so runtime z.toJSONSchema() calls are
safe.
Comment on lines
+154
to
+156
| replaceDataSourceWithRef(treeJsonSchema); | ||
| treeJsonSchema.$defs = { DataSource: dataSourceJsonSchema }; | ||
|
|
There was a problem hiding this comment.
Don't overwrite existing $defs when injecting DataSource.
z.toJSONSchema(treeSchema) can already produce $defs. Replacing the whole object here will leave dangling $refs as soon as any component prop schema emits shared definitions or recursion. Merge into the existing map instead of clobbering it.
Suggested fix
- treeJsonSchema.$defs = { DataSource: dataSourceJsonSchema };
+ treeJsonSchema.$defs = {
+ ...(treeJsonSchema.$defs as Record<string, unknown> | undefined),
+ DataSource: dataSourceJsonSchema,
+ };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-core/src/lib/generate-prompt-instructions.ts` around lines 154 -
156, The code is overwriting any existing JSON Schema definitions by replacing
treeJsonSchema.$defs; instead, preserve existing $defs and inject DataSource
into the map. Update the logic around replaceDataSourceWithRef and the
assignment to treeJsonSchema.$defs so you merge into the existing
treeJsonSchema.$defs (or create it if missing) and set only the DataSource key
to dataSourceJsonSchema, leaving other $defs intact to avoid dangling $ref
references.
Comment on lines
+5
to
+13
| function logKey(row: OtelLogsRow): string { | ||
| const body = row.Body ?? ""; | ||
| let hash = 0; | ||
| for (let i = 0; i < body.length; i++) { | ||
| hash = (hash << 5) - hash + body.charCodeAt(i); | ||
| hash = hash & hash; | ||
| } | ||
| return `${row.Timestamp}-${row.ServiceName ?? ""}-${Math.abs(hash).toString(36)}`; | ||
| } |
There was a problem hiding this comment.
Dedupe key can silently drop distinct logs.
On Line 12, the key uses a 32-bit hash of Body. Different bodies can collide under the same Timestamp + ServiceName, causing false dedupe and data loss.
💡 Safer key construction
function logKey(row: OtelLogsRow): string {
- const body = row.Body ?? "";
- let hash = 0;
- for (let i = 0; i < body.length; i++) {
- hash = (hash << 5) - hash + body.charCodeAt(i);
- hash = hash & hash;
- }
- return `${row.Timestamp}-${row.ServiceName ?? ""}-${Math.abs(hash).toString(36)}`;
+ return JSON.stringify([
+ row.Timestamp,
+ row.ServiceName ?? "",
+ row.Body ?? "",
+ ]);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-core/src/lib/log-buffer.ts` around lines 5 - 13, The dedupe key
in function logKey is using a 32-bit rolling int hash of OtelLogsRow.Body which
can collide and silently drop distinct logs; update logKey to compute a
cryptographic hash (e.g., SHA-256) of row.Body and use its hex (or base64)
output instead of the current 32-bit int so the returned key
(`${row.Timestamp}-${row.ServiceName ?? ""}-${hash}`) reliably distinguishes
different bodies; locate logKey and replace the bitwise hash logic with a secure
hash computation (using Node's crypto.createHash('sha256') or Web Crypto) while
keeping Timestamp and ServiceName in the key.
Comment on lines
+8
to
+15
| Card: { | ||
| props: z.object({ | ||
| title: z.string().nullable(), | ||
| description: z.string().nullable(), | ||
| padding: z.enum(["sm", "md", "lg"]).nullable(), | ||
| }), | ||
| hasChildren: true, | ||
| description: "A card container with optional title", |
There was a problem hiding this comment.
nullable() is making these props mandatory.
These schemas read like optional UI props, but in Zod nullable() still requires the key to exist. Because createCatalog() validates props against these exact shapes, { props: {} } will fail for components like Card, Grid, Stack, Heading, Text, Badge, and several metric widgets, and the example tree generated in this PR will not validate either. Use .nullish() or .optional().nullable() wherever omission is supposed to be allowed.
Also applies to: 18-35, 38-87, 91-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-core/src/lib/observability-catalog.ts` around lines 8 - 15, The
Zod schemas for component props (e.g., the Card.props z.object in
observability-catalog and the similar schemas for Grid, Stack, Heading, Text,
Badge, and metric widgets) use .nullable(), which still requires the key to be
present and causes validation failures when props are omitted; update these prop
fields to allow omission by using .nullish() or .optional().nullable() for each
prop type inside the props z.object used by createCatalog() so that { props: {}
} and the example tree validate correctly.
Comment on lines
+128
to
+134
| const TestRenderer = createRendererFromCatalog(_testCatalog, { | ||
| Box, | ||
| Text, | ||
| Capture, | ||
| DataComponent, | ||
| RefetchComponent, | ||
| }); |
There was a problem hiding this comment.
This file is using createRendererFromCatalog with the wrong API shape.
Per packages/ui-core/src/lib/renderer.tsx, createRendererFromCatalog() returns { Renderer, registry }; it is not itself a React component, and the second parameter is not the component registry object. That makes TestRenderer, LocalRenderer, and IntegrationRenderer objects here, so the createElement(..., { tree }) calls below will fail.
Also applies to: 260-266, 429-435, 561-564, 655-658, 671-673, 695-699, 726-731
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-core/src/lib/renderer.test.tsx` around lines 128 - 134, The test
uses createRendererFromCatalog incorrectly: it returns an object { Renderer,
registry } rather than a component, and the second argument is not the component
registry. Change each instantiation (e.g. the
TestRenderer/LocalRenderer/IntegrationRenderer assignments) to destructure the
result: const { Renderer, registry } = createRendererFromCatalog(_testCatalog,
/* options if any */); then update places that call createElement(TestRenderer,
{ tree }) to use createElement(Renderer, { tree, registry }) or otherwise supply
the returned registry to the Renderer invocation. Apply the same fix to the
other occurrences referenced (lines near 260-266, 429-435, 561-564, 655-658,
671-673, 695-699, 726-731) so tests use the Renderer component and registry
returned from createRendererFromCatalog instead of treating the call as a
component or registry argument.
Comment on lines
+26
to
+30
| type Catalog = ReturnType<typeof createCatalog>; | ||
|
|
||
| export type UITree = z.infer<Catalog["uiTreeSchema"]>; | ||
|
|
||
| type UIElement = UITree["elements"][string]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l packages/ui-core/src/lib/renderer.tsxRepository: kopai-app/kopai-mono
Length of output: 105
🏁 Script executed:
cat -n packages/ui-core/src/lib/renderer.tsx | head -50Repository: kopai-app/kopai-mono
Length of output: 1640
🏁 Script executed:
cat -n packages/ui-core/src/lib/renderer.tsx | sed -n '20,40p'Repository: kopai-app/kopai-mono
Length of output: 677
🏁 Script executed:
cat -n packages/ui-core/src/lib/renderer.tsx | sed -n '150,180p'Repository: kopai-app/kopai-mono
Length of output: 1133
🏁 Script executed:
cat -n packages/ui-core/src/lib/renderer.tsx | sed -n '320,345p'Repository: kopai-app/kopai-mono
Length of output: 1021
🏁 Script executed:
cat -n packages/ui-core/src/lib/component-catalog.ts | head -100Repository: kopai-app/kopai-mono
Length of output: 3827
🏁 Script executed:
rg "createCatalog" packages/ui-core/src/lib/ -A 10 --max-count 5Repository: kopai-app/kopai-mono
Length of output: 14927
🏁 Script executed:
rg "UITree" packages/ui-core/src/ -B 2 -A 2Repository: kopai-app/kopai-mono
Length of output: 7630
🏁 Script executed:
sed -n '200,250p' packages/ui-core/src/lib/component-catalog.tsRepository: kopai-app/kopai-mono
Length of output: 46
🏁 Script executed:
rg "return {" packages/ui-core/src/lib/component-catalog.ts -A 15 | head -40Repository: kopai-app/kopai-mono
Length of output: 143
🏁 Script executed:
sed -n '150,220p' packages/ui-core/src/lib/component-catalog.tsRepository: kopai-app/kopai-mono
Length of output: 1306
Make UITree and Renderer types generic to enforce catalog-specific tree shapes.
UITree is derived from an unbound ReturnType<typeof createCatalog> instead of the specific catalog C passed to the renderer. This allows a renderer created for one catalog to accept trees from any other catalog at compile time, only catching mismatches at runtime when the registry's prop validation fails.
Apply the suggested fix to make UITree and UIElement generic, constrained to a CatalogLike interface, and update all renderer function signatures to use UITree<C> instead of the unbound types.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-core/src/lib/renderer.tsx` around lines 26 - 30, The UITree and
UIElement types are currently unbound to a specific catalog; change them to
generics so renderer instances are catalog-safe: introduce a generic constraint
like CatalogLike and redefine UITree as UITree<C extends CatalogLike> =
z.infer<C["uiTreeSchema"]> and UIElement as UIElement<C extends CatalogLike> =
UITree<C>["elements"][string]; then update all renderer-related signatures
(e.g., the Renderer type, render function parameters, and any functions
referencing UITree or UIElement) to accept UITree<C> or UIElement<C> and ensure
createCatalog usages and type parameters propagate the specific catalog type C
through the renderer factory.
Comment on lines
+225
to
+239
| const { data, loading, error, refetch } = useKopaiData(effectiveDataSource); | ||
|
|
||
| if (!methodIsAccepted && element.dataSource) { | ||
| const accepted = acceptsDataFrom?.length | ||
| ? acceptsDataFrom.join(", ") | ||
| : "none"; | ||
| console.error( | ||
| `[ui-core] Component "${element.type}" does not accept dataSource method "${element.dataSource.method}". Accepted methods: ${accepted}.` | ||
| ); | ||
| return null; | ||
| } | ||
|
|
||
| const updateParams = useCallback((params: Record<string, unknown>) => { | ||
| setParamsOverride((prev) => ({ ...prev, ...params })); | ||
| }, []); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/ui-core/src/lib && wc -l renderer.tsxRepository: kopai-app/kopai-mono
Length of output: 80
🏁 Script executed:
cd packages/ui-core/src/lib && sed -n '200,260p' renderer.tsx | cat -nRepository: kopai-app/kopai-mono
Length of output: 2220
Move useCallback above the invalid-dataSource return.
This branch sometimes returns before calling useCallback and sometimes does not. If methodIsAccepted flips between renders, React will see a different hook order and can throw a hooks-ordering error.
Suggested fix
const { data, loading, error, refetch } = useKopaiData(effectiveDataSource);
+
+ const updateParams = useCallback((params: Record<string, unknown>) => {
+ setParamsOverride((prev) => ({ ...prev, ...params }));
+ }, []);
if (!methodIsAccepted && element.dataSource) {
const accepted = acceptsDataFrom?.length
? acceptsDataFrom.join(", ")
: "none";
console.error(
`[ui-core] Component "${element.type}" does not accept dataSource method "${element.dataSource.method}". Accepted methods: ${accepted}.`
);
return null;
}
-
- const updateParams = useCallback((params: Record<string, unknown>) => {
- setParamsOverride((prev) => ({ ...prev, ...params }));
- }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-core/src/lib/renderer.tsx` around lines 225 - 239, The hook
ordering is unstable because updateParams (created via useCallback) is declared
after a conditional early return that runs when methodIsAccepted is false; move
the useCallback declaration for updateParams (and any other hooks) above the
conditional that returns null so hooks run in the same order every render,
keeping the same dependencies (e.g., setParamsOverride) and preserving the
function name updateParams; ensure the conditional that logs the error (using
methodIsAccepted, element.dataSource, acceptsDataFrom) still returns null when
appropriate after the move.
| format: ["esm", "cjs"], | ||
| dts: true, | ||
| clean: true, | ||
| external: ["zod"], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how zod is declared in package.json
cat packages/ui-core/package.json | jq '{dependencies: .dependencies.zod, peerDependencies: .peerDependencies.zod}'Repository: kopai-app/kopai-mono
Length of output: 122
Add zod to peerDependencies in packages/ui-core/package.json.
zod is marked as external in the build config, which means it won't be bundled into the output and must be provided by consumers. While it's currently listed in dependencies, it should also be declared in peerDependencies to explicitly communicate this requirement. This aligns the package metadata with the build configuration and follows the standard pattern for external dependencies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-core/tsdown.config.ts` at line 8, The tsdown.config.ts marks
"zod" as external, so update packages/ui-core/package.json to list "zod" in
peerDependencies (e.g., a semver range like "^x.y.z") so consumers know they
must provide it; also remove it from runtime dependencies if present (or keep it
only as a devDependency for local builds/tests) and ensure the version in
peerDependencies matches the version used in devDependencies/tests; reference
the package.json’s peerDependencies section and the tsdown.config.ts external
array to make the change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
@kopai/ui-core: DOM-free UI primitives for React Native/SSR/CLI.useKopaiDataanduseLiveLogs.Documentation
@kopai/ui-core.