Implement Search and Skill detail page#5
Conversation
📝 WalkthroughWalkthroughThis PR implements a complete skill management feature spanning data operations, component primitives, and routing. It adds GraphQL mutations and paginated queries; introduces form field primitives and search UI; creates three new route pages (list with search, creation form, and detail view); updates skill card linking and root layout analytics behavior; and adds supporting styling and dependencies. ChangesSkills Management Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/dataconnect-generated/react/README.md (1)
121-124:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix reversed overload description for
useGetSkills.The sentence says you can “also pass in a
DataConnectinstance,” but the signature shown below it is the overload withoutdc. Please swap/reword to match the displayed overloads.🤖 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 `@src/dataconnect-generated/react/README.md` around lines 121 - 124, The README text states you can "also pass in a `DataConnect` instance" but the shown signature for useGetSkills is the overload that does NOT accept a `dc`; update the documentation to make the sentence and signature consistent: either change the sentence to say this signature is the overload without a DataConnect instance or replace the shown signature with the overload that accepts a `DataConnect` parameter; locate references to useGetSkills and the related types (GetSkillsVariables, useDataConnectQueryOptions<GetSkillsData>, UseDataConnectQueryResult<GetSkillsData, GetSkillsVariables>) and ensure the descriptive text matches the exact overload displayed.src/routes/__root.tsx (1)
51-71:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
PostHogAuthSyncwill fail at runtime sincePostHogProvideris commented out.With
PostHogProvidercommented out (lines 80-91),usePostHog()will likely returnundefinedor a non-functional instance. Callingposthog.has_opted_in_capturing(),posthog.identify(), orposthog.reset()will throw aTypeError.Either:
- Comment out
<PostHogAuthSync />at line 93 as well, or- Add a null guard before calling posthog methods, or
- Re-enable
PostHogProviderif analytics should remain active.🔧 Option 1: Comment out PostHogAuthSync rendering
<ClerkProvider> - <PostHogAuthSync /> + {/* <PostHogAuthSync /> */} <div className="root-layout">🔧 Option 2: Add null guard in useEffect
useEffect(() => { + if (!posthog) return; const hasAnalyticsConsent = posthog.has_opted_in_capturing(); if (!isLoaded) return;🤖 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 `@src/routes/__root.tsx` around lines 51 - 71, PostHogAuthSync calls posthog methods via usePostHog() but PostHogProvider is currently disabled, so posthog may be undefined and cause runtime TypeErrors; fix by guarding calls inside PostHogAuthSync (check posthog is truthy and methods exist before calling has_opted_in_capturing(), identify(), reset()), or remove/comment out the <PostHogAuthSync /> render, or re-enable the PostHogProvider depending on whether analytics should be active; reference the PostHogAuthSync function and the usePostHog/PostHogProvider symbols when making the change.
🧹 Nitpick comments (1)
vite.config.ts (1)
32-34: ⚡ Quick winDocument or remove the HMR overlay configuration.
The
overlay: falseconfiguration has no documented reason in comments or git history. While this may work around an issue with the devtools plugins interfering with error reporting, disabling it without explanation forces developers to check the console for errors instead of seeing them in the browser overlay. Either add a comment explaining why the overlay is disabled, or remove the configuration to restore the default error reporting behavior.🤖 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 `@vite.config.ts` around lines 32 - 34, The hmr overlay is explicitly disabled via the hmr: { overlay: false } setting; either remove that overlay property to restore Vite's default in-browser error overlay, or add a short comment above the hmr configuration explaining the precise reason (e.g., conflicts with specific devtools/plugins or false positives) and link to the related issue/PR if available; update the hmr.overlay entry inside the vite config (the hmr object) accordingly and ensure the change is committed with a brief message explaining the choice.
🤖 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 @.agents/skills/firebase-firestore/references/enterprise/provisioning.md:
- Line 81: Update the duplicate heading number for the Firestore indexes
section: locate the "Create `firestore.indexes.json`" heading (currently "###
3") and change it to "### 4" so the section numbering follows the earlier
"Create `firestore.rules`" heading and maintains correct sequential order.
In `@dataconnect/connectors/mutations.gql`:
- Around line 1-21: The CreateSkill mutation currently accepts a client-supplied
authorClerkId and is marked PUBLIC; change it to require authenticated access
and derive the author ID from the trusted server-side auth context instead of
the input. Remove the $authorClerkId variable and the authorClerkId field from
the skill_insert input, change the `@auth` directive to require authenticated
users, and in the connector/resolver that handles CreateSkill (the code path
that executes skill_insert) read the author ID from the server-side auth/claims
(e.g., currentUser.id / session.user.id) and populate the authorClerkId field
there before inserting to prevent client impersonation. Ensure
validation/logging handles missing or invalid server auth.
In `@dataconnect/connectors/queries.gql`:
- Around line 16-17: Remove PII fields from the public GraphQL skill queries by
eliminating author.email and author.clerkId from the GetSkills and GetSkillById
query selections in connectors/queries.gql; update the author selection set to
only include non-sensitive fields (e.g., username, imageUrl) or conditionally
include email/clerkId behind an authenticated/authorized query variant or
fragment if needed (refer to the GetSkills and GetSkillById query definitions
and the author { username imageUrl email clerkId } selection to locate changes).
In `@package.json`:
- Line 37: Replace the deprecated "radix-ui" meta-package with the individual
scoped packages and update imports: remove "radix-ui" from package.json and add
"`@radix-ui/react-slot`", "`@radix-ui/react-label`", and
"`@radix-ui/react-separator`"; then update all imports referencing Slot, Label,
and Separator to import from "`@radix-ui/react-slot`", "`@radix-ui/react-label`",
and "`@radix-ui/react-separator`" respectively (search for usages of Slot, Label,
Separator to locate affected files and adjust import paths).
In `@src/components/Search.tsx`:
- Around line 44-51: The search Input currently relies only on placeholder text;
update the Input component usage in the Search component to include a persistent
accessible name by adding an aria-label (e.g., aria-label="Search skills") or by
associating a visible <label> with the input; ensure you modify the Input props
where name="q", value={draftQuery}, onChange={setDraftQuery} so the accessible
name is present and announced to screen readers.
In `@src/components/ui/button.tsx`:
- Around line 41-60: The Button component currently renders a native "button"
when asChild is false without a safe default type, causing accidental form
submits; update Button so that when Comp is the native "button" it supplies
type="button" only if no type was passed in props (i.e., preserve an explicit
props.type). Locate the Button function and the Comp variable, and before
spreading {...props} ensure the rendered native button receives a default type
of "button" when asChild is false and props.type is undefined.
In `@src/components/ui/field.tsx`:
- Around line 124-133: FieldTitle is using the same slot name as FieldLabel
(data-slot="field-label"), causing slot collisions; update the FieldTitle
component to use a distinct slot name (e.g., data-slot="field-title") so
slot-based styling/selection targets the title separately; find the FieldTitle
function and replace its data-slot value, and ensure any CSS or selectors that
should target the title are updated to the new slot name to keep consistency
with FieldLabel.
In `@src/dataconnect-generated/.guides/usage.md`:
- Around line 18-23: The example incorrectly passes mutation variables into
useCreateSkill; remove variables from the hook call (call useCreateSkill() with
no args) and invoke the mutation with variables when executing (use the returned
mutation's mutate / mutateAsync method to pass createSkillVars). Specifically
update the useCreateSkill usage shown (the line with
useCreateSkill(createSkillVars)) to call useCreateSkill() and then call the
returned mutation.mutate(createSkillVars) where the creation is triggered so
TypeScript types and runtime behavior are correct.
- Around line 61-71: The usage snippet has inconsistent symbol names: you import
createSkill, getSkills, getSkillById but call CreateSkill/GetSkills/GetSkillById
and reference CreateSkillVars/GetSkillsVars/GetSkillByIdVars; update the calls
to use the imported camelCase functions (call createSkill(dataConnect,
createSkillVariables), getSkills(...), getSkillById(...)) or change the imports
to the PascalCase variants, and normalize the variable type names from the
incorrect "*Vars" to the generated "*Variables" types (e.g.,
CreateSkillVariables, GetSkillsVariables, GetSkillByIdVariables) so import
names, call sites, and type names match.
In `@src/dataconnect-generated/react/README.md`:
- Around line 226-229: The README text for useGetSkillById is reversed: the
prose claims “also pass in a DataConnect instance” but the displayed signature
is the overload without a DataConnect parameter; update the docs so the sentence
matches the shown overload—either change the sentence to say this signature is
the no-dc overload or replace the shown signature with the overload that accepts
a DataConnect instance; locate the useGetSkillById overloads in the generated
react README and make the sentence and the signature for useGetSkillById
consistent.
In `@src/routes/skills/`$skillId.tsx:
- Around line 173-176: The code renders raw PII via skill.author.email; replace
that with a non-sensitive identifier by using skill.author.username when
available or a masked email representation otherwise. Add or call a small helper
(e.g., maskEmail(email: string): string) and update the JSX that currently
references skill.author.email to use skill.author.username ||
maskEmail(skill.author.email); ensure the helper is imported/defined in the same
module and that null/undefined author or email are safely handled (fallback to
"Unknown author").
In `@src/routes/skills/new.tsx`:
- Around line 20-22: The server handler for createSkillFn is currently returning
the raw input (SubmitSkillFormValues) without runtime validation; update the
createSkillFn.handler to call submitSkillSchema.parse(data) (or parseAsync) at
the start of the handler, use the parsed result for subsequent mutation calls,
and let validation errors propagate so malicious or malformed POSTs are rejected
before any createSkill mutation runs.
---
Outside diff comments:
In `@src/dataconnect-generated/react/README.md`:
- Around line 121-124: The README text states you can "also pass in a
`DataConnect` instance" but the shown signature for useGetSkills is the overload
that does NOT accept a `dc`; update the documentation to make the sentence and
signature consistent: either change the sentence to say this signature is the
overload without a DataConnect instance or replace the shown signature with the
overload that accepts a `DataConnect` parameter; locate references to
useGetSkills and the related types (GetSkillsVariables,
useDataConnectQueryOptions<GetSkillsData>,
UseDataConnectQueryResult<GetSkillsData, GetSkillsVariables>) and ensure the
descriptive text matches the exact overload displayed.
In `@src/routes/__root.tsx`:
- Around line 51-71: PostHogAuthSync calls posthog methods via usePostHog() but
PostHogProvider is currently disabled, so posthog may be undefined and cause
runtime TypeErrors; fix by guarding calls inside PostHogAuthSync (check posthog
is truthy and methods exist before calling has_opted_in_capturing(), identify(),
reset()), or remove/comment out the <PostHogAuthSync /> render, or re-enable the
PostHogProvider depending on whether analytics should be active; reference the
PostHogAuthSync function and the usePostHog/PostHogProvider symbols when making
the change.
---
Nitpick comments:
In `@vite.config.ts`:
- Around line 32-34: The hmr overlay is explicitly disabled via the hmr: {
overlay: false } setting; either remove that overlay property to restore Vite's
default in-browser error overlay, or add a short comment above the hmr
configuration explaining the precise reason (e.g., conflicts with specific
devtools/plugins or false positives) and link to the related issue/PR if
available; update the hmr.overlay entry inside the vite config (the hmr object)
accordingly and ensure the change is committed with a brief message explaining
the choice.
🪄 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 Plus
Run ID: db97f6eb-b78e-44f1-bb1d-0104bbaf3650
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (30)
.agents/skills/firebase-firestore/references/enterprise/provisioning.md.env.example.gitignoredataconnect/connectors/mutations.gqldataconnect/connectors/queries.gqlpackage.jsonsrc/components/Search.tsxsrc/components/SkillCard.tsxsrc/components/ui/button.tsxsrc/components/ui/field.tsxsrc/components/ui/input.tsxsrc/components/ui/label.tsxsrc/components/ui/separator.tsxsrc/components/ui/textarea.tsxsrc/dataconnect-generated/.guides/usage.mdsrc/dataconnect-generated/README.mdsrc/dataconnect-generated/esm/index.esm.jssrc/dataconnect-generated/index.cjs.jssrc/dataconnect-generated/index.d.tssrc/dataconnect-generated/react/README.mdsrc/dataconnect-generated/react/esm/index.esm.jssrc/dataconnect-generated/react/index.cjs.jssrc/dataconnect-generated/react/index.d.tssrc/routeTree.gen.tssrc/routes/__root.tsxsrc/routes/skills/$skillId.tsxsrc/routes/skills/index.tsxsrc/routes/skills/new.tsxsrc/styles.cssvite.config.ts
💤 Files with no reviewable changes (1)
- .env.example
| *See [security_rules.md](security_rules.md) for how to write actual rules.* | ||
| _See [security_rules.md](security_rules.md) for how to write actual rules._ | ||
|
|
||
| ### 3. Create `firestore.indexes.json` |
There was a problem hiding this comment.
Fix duplicate section numbering.
Both "Create firestore.rules" (Line 63) and "Create firestore.indexes.json" (Line 81) are numbered as "### 3". The indexes section should be "### 4" to maintain correct sequential numbering.
📝 Proposed fix
-### 3. Create `firestore.indexes.json`
+### 4. Create `firestore.indexes.json`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### 3. Create `firestore.indexes.json` | |
| ### 4. Create `firestore.indexes.json` |
🤖 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 @.agents/skills/firebase-firestore/references/enterprise/provisioning.md at
line 81, Update the duplicate heading number for the Firestore indexes section:
locate the "Create `firestore.indexes.json`" heading (currently "### 3") and
change it to "### 4" so the section numbering follows the earlier "Create
`firestore.rules`" heading and maintains correct sequential order.
| mutation CreateSkill( | ||
| $authorClerkId: String!, | ||
| $title: String!, | ||
| $description: String!, | ||
| $tags: [String!]!, | ||
| $installCommand: String!, | ||
| $promptConfig: String!, | ||
| $usageExample: String! | ||
| ) @auth(level: PUBLIC insecureReason: "Clerk auth is handled on the frontend") { | ||
| skill_insert( | ||
| data: { | ||
| authorClerkId: $authorClerkId | ||
| title: $title | ||
| description: $description | ||
| tags: $tags | ||
| installCommand: $installCommand | ||
| promptConfig: $promptConfig | ||
| usageExample: $usageExample | ||
| } | ||
| ) | ||
| } No newline at end of file |
There was a problem hiding this comment.
Block public write + client-controlled author identity.
This mutation allows unauthenticated callers to create skills and impersonate any author by sending any authorClerkId. Enforce authenticated access at the connector level and derive author identity from trusted auth context/server-side claims instead of mutation input.
🤖 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 `@dataconnect/connectors/mutations.gql` around lines 1 - 21, The CreateSkill
mutation currently accepts a client-supplied authorClerkId and is marked PUBLIC;
change it to require authenticated access and derive the author ID from the
trusted server-side auth context instead of the input. Remove the $authorClerkId
variable and the authorClerkId field from the skill_insert input, change the
`@auth` directive to require authenticated users, and in the connector/resolver
that handles CreateSkill (the code path that executes skill_insert) read the
author ID from the server-side auth/claims (e.g., currentUser.id /
session.user.id) and populate the authorClerkId field there before inserting to
prevent client impersonation. Ensure validation/logging handles missing or
invalid server auth.
| author { username imageUrl clerkId email } | ||
| } |
There was a problem hiding this comment.
Avoid exposing author email and clerkId in public queries.
GetSkills/GetSkillById are public, but they return direct identifiers (email, clerkId). This is unnecessary PII exposure for browse/detail endpoints; trim these fields or gate them behind stricter auth.
Also applies to: 22-23
🤖 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 `@dataconnect/connectors/queries.gql` around lines 16 - 17, Remove PII fields
from the public GraphQL skill queries by eliminating author.email and
author.clerkId from the GetSkills and GetSkillById query selections in
connectors/queries.gql; update the author selection set to only include
non-sensitive fields (e.g., username, imageUrl) or conditionally include
email/clerkId behind an authenticated/authorized query variant or fragment if
needed (refer to the GetSkills and GetSkillById query definitions and the author
{ username imageUrl email clerkId } selection to locate changes).
| "nitro": "npm:nitro-nightly@latest", | ||
| "posthog-js": "^1.373.4", | ||
| "posthog-node": "^5.34.1", | ||
| "radix-ui": "^1.4.3", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does npm package radix-uiversion1.4.3export React primitives likeSlot, Label, and Separatorfromimport { ... } from "radix-ui"? If not, what are the correct package names/import paths for these primitives?
💡 Result:
The npm package radix-ui (version 1.4.3) is a legacy meta-package that is no longer the recommended way to consume Radix Primitives [1][2][3]. While some older documentation may suggest importing components directly from radix-ui [4][5], this approach is deprecated in favor of installing and importing individual primitive packages [3][6]. To use Slot, Label, and Separator, you should install their specific packages and import them from those respective paths: 1. Slot: Install @radix-ui/react-slot [4] and import via import { Slot } from "@radix-ui/react-slot";. 2. Label: Install @radix-ui/react-label and import via import * as Label from "@radix-ui/react-label"; (or specific exports if available). 3. Separator: Install @radix-ui/react-separator [7] and import via import * as Separator from "@radix-ui/react-separator"; [7]. The Radix team recommends installing individual packages to ensure better tree-shaking, manage dependencies more effectively, and prevent version conflicts [3][6]. Using the monolithic radix-ui package is discouraged for modern projects [3].
Citations:
- 1: https://www.npmjs.com/package/radix-ui
- 2: https://www.npmjs.com/package/radix-ui?activeTab=versions
- 3: https://www.radix-ui.com/docs/primitives
- 4: https://www.radix-ui.com/primitives/docs/utilities/slot
- 5: https://www.radix-ui.com/primitives/docs/overview/getting-started
- 6: https://mintlify.com/radix-ui/primitives/installation
- 7: https://www.radix-ui.com/primitives/docs/components/separator
🏁 Script executed:
# Search for imports from "radix-ui" in the codebase
rg 'from\s+["\']radix-ui["\']' --type ts --type tsx --type js --type jsx -nRepository: JUGALTYAGI/Tanstack-skilled-project
Length of output: 148
🏁 Script executed:
# Search for imports from "radix-ui" in the codebase
rg 'from.*radix-ui' --type ts --type tsx --type js --type jsx -nRepository: JUGALTYAGI/Tanstack-skilled-project
Length of output: 107
🏁 Script executed:
# Search for imports from "radix-ui" in the codebase
rg 'from.*radix-ui' -nRepository: JUGALTYAGI/Tanstack-skilled-project
Length of output: 316
🏁 Script executed:
# Check package.json for dependency details
cat package.json | grep -A 30 '"dependencies"'Repository: JUGALTYAGI/Tanstack-skilled-project
Length of output: 1162
🌐 Web query:
What does the npm package "radix-ui" version 1.4.3 actually export? Does it have Slot, Separator, and Label exports?
💡 Result:
The npm package radix-ui (version 1.4.3) is a unified package that serves as a wrapper or entry point for Radix Primitives [1][2]. It is designed to allow users to import various components directly from a single package [3]. Regarding your specific questions: 1. What it exports: The radix-ui package uses an exports map in its package.json to define its entry points [4]. It is intended to provide access to the various Radix UI primitives [1][3]. 2. Does it have Slot, Separator, and Label exports: Yes, the radix-ui package is designed to expose these components [3]. In the Radix UI ecosystem, Slot, Separator, and Label are standard primitives [5]. When using the unified radix-ui package, you typically import these components directly from it (e.g., import { Slot, Separator, Label } from "radix-ui") [3]. Note that while the unified radix-ui package exists, Radix UI components are also frequently consumed via their individual scoped packages (e.g., @radix-ui/react-slot, @radix-ui/react-separator, and @radix-ui/react-label) [4][5]. The unified package simplifies installation and imports for many users [3]. If you encounter issues resolving files within this package, it is often due to the specific configuration of its exports map [4].
Citations:
- 1: https://npmx.dev/package/radix-ui/v/1.4.3
- 2: https://www.npmjs.com/package/radix-ui
- 3: https://www.radix-ui.com/primitives/docs/overview/getting-started
- 4: radix-ui package exports map prevents resolving package.json radix-ui/primitives#3818
- 5: https://github.com/radix-ui/primitives/tree/main/packages
Use individual Radix packages instead of the deprecated radix-ui meta-package.
The imports from "radix-ui"@1.4.3` will technically resolve (Slot, Separator, and Label are exposed), but this is a deprecated meta-package. Radix recommends installing individual scoped packages for better tree-shaking and dependency management. Replace with:
@radix-ui/react-slotfor Slot@radix-ui/react-labelfor Label@radix-ui/react-separatorfor Separator
Update package.json and adjust imports accordingly.
🤖 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 `@package.json` at line 37, Replace the deprecated "radix-ui" meta-package with
the individual scoped packages and update imports: remove "radix-ui" from
package.json and add "`@radix-ui/react-slot`", "`@radix-ui/react-label`", and
"`@radix-ui/react-separator`"; then update all imports referencing Slot, Label,
and Separator to import from "`@radix-ui/react-slot`", "`@radix-ui/react-label`",
and "`@radix-ui/react-separator`" respectively (search for usages of Slot, Label,
Separator to locate affected files and adjust import paths).
| <Input | ||
| name="q" | ||
| type="text" | ||
| value={draftQuery} | ||
| onChange={(event) => setDraftQuery(event.target.value)} | ||
| placeholder="Search skills..." | ||
| className="search-input" | ||
| /> |
There was a problem hiding this comment.
Add an explicit accessible name to the search input.
Line 44-51 relies on placeholder text only. Please add a persistent accessible label (aria-label or <label> association) for screen reader usability.
Proposed fix
<Input
name="q"
type="text"
value={draftQuery}
onChange={(event) => setDraftQuery(event.target.value)}
+ aria-label="Search skills"
placeholder="Search skills..."
className="search-input"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Input | |
| name="q" | |
| type="text" | |
| value={draftQuery} | |
| onChange={(event) => setDraftQuery(event.target.value)} | |
| placeholder="Search skills..." | |
| className="search-input" | |
| /> | |
| <Input | |
| name="q" | |
| type="text" | |
| value={draftQuery} | |
| onChange={(event) => setDraftQuery(event.target.value)} | |
| aria-label="Search skills" | |
| placeholder="Search skills..." | |
| className="search-input" | |
| /> |
🤖 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 `@src/components/Search.tsx` around lines 44 - 51, The search Input currently
relies only on placeholder text; update the Input component usage in the Search
component to include a persistent accessible name by adding an aria-label (e.g.,
aria-label="Search skills") or by associating a visible <label> with the input;
ensure you modify the Input props where name="q", value={draftQuery},
onChange={setDraftQuery} so the accessible name is present and announced to
screen readers.
| const { data, isPending, isSuccess, isError, error } = useCreateSkill(createSkillVars); | ||
|
|
||
| const { data, isPending, isSuccess, isError, error } = useGetSkills(getSkillsVars); | ||
|
|
||
| const { data, isPending, isSuccess, isError, error } = useGetSkillById(getSkillByIdVars); | ||
|
|
There was a problem hiding this comment.
useCreateSkill example uses the mutation hook incorrectly.
useCreateSkill should not be called with mutation variables; variables belong in mutation.mutate(...). Current snippet will mislead users and can cause TS errors.
Suggested doc fix
-const { data, isPending, isSuccess, isError, error } = useCreateSkill(createSkillVars);
+const { data, isPending, isSuccess, isError, error, mutate } = useCreateSkill();
+mutate(createSkillVars);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data, isPending, isSuccess, isError, error } = useCreateSkill(createSkillVars); | |
| const { data, isPending, isSuccess, isError, error } = useGetSkills(getSkillsVars); | |
| const { data, isPending, isSuccess, isError, error } = useGetSkillById(getSkillByIdVars); | |
| const { data, isPending, isSuccess, isError, error, mutate } = useCreateSkill(); | |
| mutate(createSkillVars); | |
| const { data, isPending, isSuccess, isError, error } = useGetSkills(getSkillsVars); | |
| const { data, isPending, isSuccess, isError, error } = useGetSkillById(getSkillByIdVars); |
🤖 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 `@src/dataconnect-generated/.guides/usage.md` around lines 18 - 23, The example
incorrectly passes mutation variables into useCreateSkill; remove variables from
the hook call (call useCreateSkill() with no args) and invoke the mutation with
variables when executing (use the returned mutation's mutate / mutateAsync
method to pass createSkillVars). Specifically update the useCreateSkill usage
shown (the line with useCreateSkill(createSkillVars)) to call useCreateSkill()
and then call the returned mutation.mutate(createSkillVars) where the creation
is triggered so TypeScript types and runtime behavior are correct.
| import { createSkill, getSkills, getSkillById } from '@dataconnect/generated'; | ||
|
|
||
|
|
||
| // Operation CreateSkill: For variables, look at type CreateSkillVars in ../index.d.ts | ||
| const { data } = await CreateSkill(dataConnect, createSkillVars); | ||
|
|
||
| // Operation GetSkills: For variables, look at type GetSkillsVars in ../index.d.ts | ||
| const { data } = await GetSkills(dataConnect, getSkillsVars); | ||
|
|
||
| // Operation GetSkillById: For variables, look at type GetSkillByIdVars in ../index.d.ts | ||
| const { data } = await GetSkillById(dataConnect, getSkillByIdVars); |
There was a problem hiding this comment.
Advanced usage snippet has symbol-name mismatches that won’t run.
You import createSkill/getSkills/getSkillById but call CreateSkill/GetSkills/GetSkillById. Also, the comment type names (*Vars) appear inconsistent with the generated *Variables pattern used elsewhere.
Suggested doc fix
import { createSkill, getSkills, getSkillById } from '`@dataconnect/generated`';
-// Operation CreateSkill: For variables, look at type CreateSkillVars in ../index.d.ts
-const { data } = await CreateSkill(dataConnect, createSkillVars);
+// Operation createSkill: For variables, look at type CreateSkillVariables in ../index.d.ts
+const { data } = await createSkill(dataConnect, createSkillVars);
-// Operation GetSkills: For variables, look at type GetSkillsVars in ../index.d.ts
-const { data } = await GetSkills(dataConnect, getSkillsVars);
+// Operation getSkills: For variables, look at type GetSkillsVariables in ../index.d.ts
+const { data } = await getSkills(dataConnect, getSkillsVars);
-// Operation GetSkillById: For variables, look at type GetSkillByIdVars in ../index.d.ts
-const { data } = await GetSkillById(dataConnect, getSkillByIdVars);
+// Operation getSkillById: For variables, look at type GetSkillByIdVariables in ../index.d.ts
+const { data } = await getSkillById(dataConnect, getSkillByIdVars);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { createSkill, getSkills, getSkillById } from '@dataconnect/generated'; | |
| // Operation CreateSkill: For variables, look at type CreateSkillVars in ../index.d.ts | |
| const { data } = await CreateSkill(dataConnect, createSkillVars); | |
| // Operation GetSkills: For variables, look at type GetSkillsVars in ../index.d.ts | |
| const { data } = await GetSkills(dataConnect, getSkillsVars); | |
| // Operation GetSkillById: For variables, look at type GetSkillByIdVars in ../index.d.ts | |
| const { data } = await GetSkillById(dataConnect, getSkillByIdVars); | |
| import { createSkill, getSkills, getSkillById } from '`@dataconnect/generated`'; | |
| // Operation createSkill: For variables, look at type CreateSkillVariables in ../index.d.ts | |
| const { data } = await createSkill(dataConnect, createSkillVars); | |
| // Operation getSkills: For variables, look at type GetSkillsVariables in ../index.d.ts | |
| const { data } = await getSkills(dataConnect, getSkillsVars); | |
| // Operation getSkillById: For variables, look at type GetSkillByIdVariables in ../index.d.ts | |
| const { data } = await getSkillById(dataConnect, getSkillByIdVars); |
🤖 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 `@src/dataconnect-generated/.guides/usage.md` around lines 61 - 71, The usage
snippet has inconsistent symbol names: you import createSkill, getSkills,
getSkillById but call CreateSkill/GetSkills/GetSkillById and reference
CreateSkillVars/GetSkillsVars/GetSkillByIdVars; update the calls to use the
imported camelCase functions (call createSkill(dataConnect,
createSkillVariables), getSkills(...), getSkillById(...)) or change the imports
to the PascalCase variants, and normalize the variable type names from the
incorrect "*Vars" to the generated "*Variables" types (e.g.,
CreateSkillVariables, GetSkillsVariables, GetSkillByIdVariables) so import
names, call sites, and type names match.
| You can also pass in a `DataConnect` instance to the Query hook function. | ||
| ```javascript | ||
| useGetSkillById(vars: GetSkillByIdVariables, options?: useDataConnectQueryOptions<GetSkillByIdData>): UseDataConnectQueryResult<GetSkillByIdData, GetSkillByIdVariables>; | ||
| ``` |
There was a problem hiding this comment.
Fix reversed overload description for useGetSkillById.
Same issue here: the text says “also pass in a DataConnect instance,” but the shown overload is the one without dc.
🤖 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 `@src/dataconnect-generated/react/README.md` around lines 226 - 229, The README
text for useGetSkillById is reversed: the prose claims “also pass in a
DataConnect instance” but the displayed signature is the overload without a
DataConnect parameter; update the docs so the sentence matches the shown
overload—either change the sentence to say this signature is the no-dc overload
or replace the shown signature with the overload that accepts a DataConnect
instance; locate the useGetSkillById overloads in the generated react README and
make the sentence and the signature for useGetSkillById consistent.
| <p> | ||
| <CircleUser size={14} /> | ||
| <span>{skill.author.email}</span> | ||
| </p> |
There was a problem hiding this comment.
Avoid exposing raw author email on a public detail page.
Line 175 renders PII directly. Prefer a non-sensitive identifier (e.g., username) or a masked email representation.
Proposed fix
<p>
<CircleUser size={14} />
- <span>{skill.author.email}</span>
+ <span>{skill.author.username ?? "Unknown author"}</span>
</p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <p> | |
| <CircleUser size={14} /> | |
| <span>{skill.author.email}</span> | |
| </p> | |
| <p> | |
| <CircleUser size={14} /> | |
| <span>{skill.author.username ?? "Unknown author"}</span> | |
| </p> |
🤖 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 `@src/routes/skills/`$skillId.tsx around lines 173 - 176, The code renders raw
PII via skill.author.email; replace that with a non-sensitive identifier by
using skill.author.username when available or a masked email representation
otherwise. Add or call a small helper (e.g., maskEmail(email: string): string)
and update the JSX that currently references skill.author.email to use
skill.author.username || maskEmail(skill.author.email); ensure the helper is
imported/defined in the same module and that null/undefined author or email are
safely handled (fallback to "Unknown author").
| export const createSkillFn = createServerFn({ method: "POST" }) | ||
| .inputValidator((d: SubmitSkillFormValues) => d) | ||
| .handler(async ({ data }) => { |
There was a problem hiding this comment.
Enforce schema validation in the server function.
Line 21 currently echoes input without runtime validation, so direct POST requests can bypass client-side constraints. Validate with submitSkillSchema.parse(...) in the server path before mutation execution.
🤖 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 `@src/routes/skills/new.tsx` around lines 20 - 22, The server handler for
createSkillFn is currently returning the raw input (SubmitSkillFormValues)
without runtime validation; update the createSkillFn.handler to call
submitSkillSchema.parse(data) (or parseAsync) at the start of the handler, use
the parsed result for subsequent mutation calls, and let validation errors
propagate so malicious or malformed POSTs are rejected before any createSkill
mutation runs.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Chores