fix: Resolve Widespread TypeScript errors in template data structures#256
Merged
Conversation
- Refactor TemplateFile, TemplateFolder, and TemplateItem types to use a unified BaseTemplateItem with an optional content property. - Fix Prisma InputJsonValue casting in playground actions. - Resolve missing properties and typing issues in EnvManager tests. - Fix useWebContainer hook signature in playground page. - Remove duplicate imports and fix WebContainer event listener typing. - Ensure all tests pass and tsc --noEmit reports zero errors.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
👋 Thanks for opening a PR, @Srushti-Kamble14!Your PR has entered the 🚦 PR Review Pipeline.
What happens next
A pipeline status comment will appear below and update automatically as your PR progresses. While you wait
This comment is posted only once. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR consolidates the template type model to support optional content, exports reusable chat tool input schemas, adds content normalization during persistence, updates test validation logic and fixtures, and refactors the NPM search endpoint to use a shared constant. ChangesTemplate types, validation, persistence, and related wiring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
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 unit tests (beta)
Comment |
Srushti-Kamble14
changed the title
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/playground/components/package-manager.tsx (1)
87-93:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard non-OK NPM search responses before using
data.objects
searchNpmreadsdata.objectsunconditionally; on 4xx/5xx or rate-limit payloads whereobjectsis missing,searchResults.length/searchResults.map(...)can crash rendering. Checkres.okand fall back to an empty array (also clear results on failure).Suggested fix
const res = await fetch( `${NPM_REGISTRY_SEARCH_URL}?text=${encodeURIComponent(searchQuery)}&size=10` ); - - const data = await res.json(); - setSearchResults(data.objects); + if (!res.ok) { + throw new Error(`NPM search failed with status ${res.status}`); + } + const data: Partial<NpmSearchResult> = await res.json(); + setSearchResults(Array.isArray(data.objects) ? data.objects : []); } catch (_error) { + setSearchResults([]); toast.error("Failed to search NPM registry");🤖 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 `@modules/playground/components/package-manager.tsx` around lines 87 - 93, The NPM search handler currently assigns setSearchResults(data.objects) unconditionally which will crash if the registry returns a non-OK response or a payload without objects; update the fetch logic in the search function (where NPM_REGISTRY_SEARCH_URL, searchQuery and setSearchResults are used) to first check res.ok, then parse JSON and setSearchResults to data.objects || [] on success, and on non-ok responses or exceptions ensure you call setSearchResults([]) (and optionally log the error) so the component clears results instead of crashing.
🧹 Nitpick comments (2)
app/api/upload-zip/route.ts (1)
14-24: ⚡ Quick winUse the shared template types instead of redefining them locally.
This route now has another copy of
TemplateFolder/TemplateFile, which can drift again. Prefer importing the canonical types to keep contracts aligned.💡 Proposed refactor
import { Prisma } from "`@prisma/client`"; +import type { TemplateFile, TemplateFolder } from "`@/modules/playground/lib/path-to-json`"; -interface TemplateFile { - filename: string; - fileExtension: string; - content: string; -} - -interface TemplateFolder { - folderName: string; - items: (TemplateFile | TemplateFolder)[]; - content?: string; -}🤖 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 `@app/api/upload-zip/route.ts` around lines 14 - 24, Remove the local duplicate interfaces TemplateFile and TemplateFolder from route.ts and import the canonical types from the shared template types module instead; replace the local declarations with an import (use the shared export name(s) TemplateFile and TemplateFolder) and ensure any usages in functions (e.g., the upload/zip route handlers) reference the imported types so the file compiles against the single source-of-truth.tests/env-manager.test.tsx (1)
42-42: ⚡ Quick winAvoid
as anyforinstancetest prop to preserve contract checks.At Line 42 (and similarly Lines 55, 74, 99, 126),
as anyremoves useful compile-time protection in these tests. Prefer a minimal typed mock (e.g., usingReact.ComponentProps<typeof EnvManager>["instance"]) so test fixtures fail when the component contract changes.Also applies to: 55-55, 74-74, 99-99, 126-126
🤖 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 `@tests/env-manager.test.tsx` at line 42, Replace the unsafe "as any" casts used for the instance prop in the EnvManager tests: instead of "instance={instanceMock as any}", type the mock using the component's prop type (e.g., React.ComponentProps<typeof EnvManager>["instance"]) and declare instanceMock with that type so the tests retain compile-time contract checks; update all occurrences (the instance prop usages and the instanceMock declaration) to use this typed mock rather than as any.
🤖 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 `@app/api/chat.test.ts`:
- Line 18: The tests use an `as any` cast on tools.edit_file.inputSchema and
tools.edit_multiple_files.inputSchema before calling safeParse; remove the casts
and use the real Zod-derived types instead (or rely on the schema's existing
typing) so safeParse is called with properly typed input. Replace occurrences of
`(tools.edit_file.inputSchema as any).safeParse(...)` and
`(tools.edit_multiple_files.inputSchema as any).safeParse(...)` in
app/api/chat.test.ts with either direct
`tools.edit_file.inputSchema.safeParse(...)` /
`tools.edit_multiple_files.inputSchema.safeParse(...)` or use z.infer<typeof
tools.edit_file.inputSchema> (and similarly for edit_multiple_files) to type the
test input variables.
In `@modules/playground/actions/index.ts`:
- Line 176: Replace the unsafe double-cast of `data as unknown as
Prisma.InputJsonValue` used when setting the `content` field with a JSON-safe
normalization step: implement and call a helper (e.g. `normalizeJsonForPrisma`
or `sanitizeJsonInput`) that recursively removes any `undefined` values,
converts JS `null`/empty values to explicit `Prisma.JsonNull` or `Prisma.DbNull`
where appropriate, and returns a value that conforms to `Prisma.InputJsonValue`;
then use that normalized result for the `content` assignment (and the other
occurrence) instead of the cast.
---
Outside diff comments:
In `@modules/playground/components/package-manager.tsx`:
- Around line 87-93: The NPM search handler currently assigns
setSearchResults(data.objects) unconditionally which will crash if the registry
returns a non-OK response or a payload without objects; update the fetch logic
in the search function (where NPM_REGISTRY_SEARCH_URL, searchQuery and
setSearchResults are used) to first check res.ok, then parse JSON and
setSearchResults to data.objects || [] on success, and on non-ok responses or
exceptions ensure you call setSearchResults([]) (and optionally log the error)
so the component clears results instead of crashing.
---
Nitpick comments:
In `@app/api/upload-zip/route.ts`:
- Around line 14-24: Remove the local duplicate interfaces TemplateFile and
TemplateFolder from route.ts and import the canonical types from the shared
template types module instead; replace the local declarations with an import
(use the shared export name(s) TemplateFile and TemplateFolder) and ensure any
usages in functions (e.g., the upload/zip route handlers) reference the imported
types so the file compiles against the single source-of-truth.
In `@tests/env-manager.test.tsx`:
- Line 42: Replace the unsafe "as any" casts used for the instance prop in the
EnvManager tests: instead of "instance={instanceMock as any}", type the mock
using the component's prop type (e.g., React.ComponentProps<typeof
EnvManager>["instance"]) and declare instanceMock with that type so the tests
retain compile-time contract checks; update all occurrences (the instance prop
usages and the instanceMock declaration) to use this typed mock rather than as
any.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f991fd7-1318-46eb-8cc0-f59f0e53aced
📒 Files selected for processing (11)
app/api/chat.test.tsapp/api/upload-zip/route.tsapp/playground/[id]/page.tsxmodules/dashboard/components/template-selecting-modal.tsxmodules/playground/actions/index.tsmodules/playground/components/ai-chat-panel.tsxmodules/playground/components/explorer-dialogs/types.tsmodules/playground/components/package-manager.tsxmodules/playground/lib/path-to-json.tsmodules/webcontainers/components/webcontainer-preview.tsxtests/env-manager.test.tsx
💤 Files with no reviewable changes (1)
- modules/dashboard/components/template-selecting-modal.tsx
piyushdotcomm
added
good first issue
Contributor
Author
|
Hi @piyushdotcomm , Requested changes fixed:
|
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
2 participants
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
What changed:
- Refactored TemplateFile, TemplateFolder, and TemplateItem types into a unified structure using a BaseTemplateItem interface with an optional
content property.
- Fixed Prisma InputJsonValue casting in modules/playground/actions/index.ts using safe unknown transitions.
- Corrected the useWebContainer hook signature in app/playground/[id]/page.tsx (removed unnecessary arguments).
- Resolved duplicate imports and event listener typing issues in WebContainerPreview.
- Added missing toast import in PackageManager.
- Updated EnvManager tests with missing required properties (folderName) and resolved DOM/Instance typing conflicts.
Why it changed:
- The previous type definitions caused widespread "Property 'content' missing" errors when accessing TemplateItem unions.
- Inconsistent hook usage and duplicate imports were blocking successful production builds and causing IDE noise.
- Test suites were failing due to outdated mock structures following recent schema changes.
Type of change
Related issue
Closes #245
Validation
npm run lintnpm testnpm run buildList any additional manual verification you performed:
Checklist
Summary by CodeRabbit
Bug Fixes
Chores