fix: file upload build error #2674
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughUpload handling updated to detect binary vs text files, choose smarter target directories (images: public → src/assets → assets; non-images: active file directory or root), write binaries with writeBinaryFile and texts with writeFile, add safer file selection checks, improved error handling, and reset uploading state. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant M as UploadModal
participant FS as FileSystem
U->>M: Select file(s)
M->>M: Ensure fileArray[0] exists
M->>M: getSmartDirectory(file)
alt isBinaryFile(file.name)
M->>FS: writeBinaryFile(path, Uint8Array)
else
M->>FS: writeFile(path, file.text())
end
M->>FS: listAllFiles()
M->>U: Show success toast
alt Error
M->>U: Show failure toast
end
M->>M: set isUploading = false
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx (1)
53-68: Potential logic issue in image directory fallbackLine 67 contains redundant logic - it checks for 'public' again after already checking it on line 58. The fallback should likely default to 'root' for images when no standard directories exist.
- return availableDirectories.includes('public') ? 'public' : 'root'; + return 'root';
🧹 Nitpick comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx (1)
110-116: Consider adding MIME type validation for safer file handlingWhile checking
file.type.startsWith('image/')works for most cases, some files might have incorrect or missing MIME types. Consider adding a fallback check based on file extension.- if (file.type.startsWith('image/')) { + const extension = file.name.toLowerCase().split('.').pop(); + const imageExtensions = ['jpg', 'jpeg', 'png', 'gif', 'svg', 'webp', 'ico']; + const isImage = file.type.startsWith('image/') || + (extension && imageExtensions.includes(extension)); + if (isImage) { const content = await file.arrayBuffer(); await editorEngine.sandbox.writeBinaryFile(finalPath, new Uint8Array(content)); } else {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx(6 hunks)
🔇 Additional comments (3)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx (3)
93-96: Good defensive programming with the file existence checkThe added check
fileArray[0]prevents potential undefined access when computing the smart directory, improving robustness against edge cases.
110-116: Correct file handling approach for different content typesThe differentiation between binary (images) and text files is appropriate. Using
writeBinaryFilefor images withUint8ArrayandwriteFilefor text content ensures proper handling of different file types and prevents potential encoding issues.
127-132: Excellent error handling improvementsThe addition of try-catch-finally ensures proper error handling with user feedback via toast notifications and guarantees the loading state is reset even on failure. This prevents the UI from getting stuck in a loading state.
| if (file.type.startsWith('image/')) { | ||
| const content = await file.arrayBuffer(); | ||
| await editorEngine.sandbox.writeBinaryFile(finalPath, new Uint8Array(content)); | ||
| } else { | ||
| const content = await file.text(); | ||
| await editorEngine.sandbox.writeFile(finalPath, content); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding file size validation
For large files, loading the entire content into memory via arrayBuffer() or text() could cause performance issues or browser crashes. Consider adding file size validation before processing.
Add size validation before the upload loop:
const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB limit
const oversizedFiles = selectedFiles.filter(file => file.size > MAX_FILE_SIZE);
if (oversizedFiles.length > 0) {
toast('Files too large', {
description: `The following files exceed the 10MB limit: ${oversizedFiles.map(f => f.name).join(', ')}`
});
return;
}🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx
around lines 110-116, add file size validation before calling file.arrayBuffer()
or file.text(): define a MAX_FILE_SIZE (e.g. 10 * 1024 * 1024), check
selectedFiles for any file.size > MAX_FILE_SIZE, and if any are oversized show a
toast listing their names and stop the upload (or skip oversized files) so you
never load large files fully into memory; ensure the check runs before the
upload loop and that the UI informs the user which files exceeded the limit.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx (1)
100-107: Add file size validation before reading file contents into memoryReading large files with file.arrayBuffer() or file.text() can blow memory or freeze the UI. Add a size cap and notify the user before the upload loop.
const handleUpload = async () => { if (selectedFiles.length === 0) { return; } + // Prevent loading very large files into memory + const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB + const oversized = selectedFiles.filter(f => f.size > MAX_FILE_SIZE); + if (oversized.length > 0) { + toast('Files too large', { + description: `The following files exceed the 10MB limit: ${oversized.map(f => f.name).join(', ')}` + }); + return; + } + setIsUploading(true);
🧹 Nitpick comments (3)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx (3)
54-69: Broaden image extension coverage and remove redundant fallback
- The image extension list misses common types (avif, tif/tiff, bmp, apng, heic/heif). This can misclassify image files and degrade the “smart” directory selection.
- The final return in the image branch repeats the 'public' check and is effectively equivalent to returning 'root' at that point.
Consider this focused tweak:
- const imageExtensions = ['jpg', 'jpeg', 'png', 'gif', 'svg', 'webp', 'ico']; + const imageExtensions = [ + 'jpg', 'jpeg', 'png', 'gif', 'svg', 'webp', 'ico', + 'avif', 'tif', 'tiff', 'bmp', 'apng', 'heic', 'heif' + ]; @@ - return availableDirectories.includes('public') ? 'public' : 'root'; + return 'root';
71-81: Prefer the nearest existing parent directory over falling back to rootIf the active file’s directory isn’t in availableDirectories, you immediately return 'root'. Often a parent directory exists (e.g., active dir "src/pages" while only "src" is present). Select the deepest existing parent to improve UX.
- if (editorEngine.ide.activeFile?.path) { + if (editorEngine.ide.activeFile?.path) { const path = editorEngine.ide.activeFile.path; const lastSlash = path.lastIndexOf('/'); const dir = lastSlash > 0 ? path.substring(0, lastSlash) : 'root'; - if (dir === 'root' || availableDirectories.includes(dir)) { - return dir; - } + if (dir === 'root' || availableDirectories.includes(dir)) { + return dir; + } + // Try nearest existing parent of the active file's directory + const parts = dir.split('/'); + for (let i = parts.length - 1; i > 0; i--) { + const candidate = parts.slice(0, i).join('/'); + if (availableDirectories.includes(candidate)) { + return candidate; + } + } }
128-133: Consider per-file error handling to avoid failing the whole batchA single failing file aborts the entire upload. If that’s not desired, wrap the body of the for-of loop in its own try/catch, collect errors, and present a summary toast while still uploading the rest.
- try { - for (const file of selectedFiles) { + try { + const failures: string[] = []; + for (const file of selectedFiles) { + try { const directory = targetDirectory === 'root' ? '' : targetDirectory; const finalPath = directory ? `${directory}/${file.name}` : file.name; if (isBinaryFile(file.name)) { const content = await file.arrayBuffer(); await editorEngine.sandbox.writeBinaryFile(finalPath, new Uint8Array(content)); } else { const content = await file.text(); await editorEngine.sandbox.writeFile(finalPath, content); } + } catch (e) { + console.error('Error uploading file:', file.name, e); + failures.push(file.name); + } } await editorEngine.sandbox.listAllFiles(); - const fileCount = selectedFiles.length; - const fileText = fileCount === 1 ? selectedFiles[0]?.name ?? 'file' : `${fileCount} files`; - toast(`Successfully uploaded ${fileText}!`); + const fileCount = selectedFiles.length; + if (failures.length === 0) { + const fileText = fileCount === 1 ? selectedFiles[0]?.name ?? 'file' : `${fileCount} files`; + toast(`Successfully uploaded ${fileText}!`); + } else if (failures.length < fileCount) { + toast('Partial upload completed', { + description: `Failed: ${failures.join(', ')}` + }); + } else { + throw new Error('All uploads failed'); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx (2)
packages/utility/src/file.ts (1)
isBinaryFile(9-12)packages/ui/src/components/sonner.tsx (1)
toast(12-12)
🔇 Additional comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx (1)
94-97: Good null guard on first selected fileThanks for guarding against an empty selection before computing the smart directory.
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx
Outdated
Show resolved
Hide resolved
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/web/client/src/components/store/editor/image/index.ts (1)
264-266: Type narrowing bug: filter(Boolean) does not narrow to ImageContentData[]results is (ImageContentData | null)[]; filter((r) => !!r) produces (ImageContentData | null)[], which does not match the declared return Promise<ImageContentData[]> under strict typing. Use a type predicate to narrow:
- const validImages = results.filter((result) => !!result); - return validImages; + const validImages = results.filter( + (result): result is ImageContentData => !!result + ); + return validImages;
🧹 Nitpick comments (1)
apps/web/client/src/components/store/editor/image/index.ts (1)
193-196: Avoid stale state when image folder yields no resultsIf files is falsy, _imagePaths remains unchanged, potentially leaving stale entries. Clear it in this branch as done for the empty-array case.
- if (!files) { - console.error('No files found in image folder'); - return; - } + if (!files) { + console.error('No files found in image folder'); + this._imagePaths = []; + return; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx(7 hunks)apps/web/client/src/components/store/editor/image/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/client/src/app/project/[id]/_components/right-panel/dev-tab/upload-modal.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web/client/src/components/store/editor/image/index.ts (1)
packages/models/src/actions/index.ts (1)
ActionTarget(8-12)
🔇 Additional comments (1)
apps/web/client/src/components/store/editor/image/index.ts (1)
173-179: No ActionTarget mismatch—code is correctThe
ActionTargetinterface inpackages/models/src/actions/target.tsis defined as:export interface ActionTarget { domId: string; oid: string | null; frameId: string; }Your code already constructs exactly that shape (
frameId,domId,oid). No remapping or renaming is needed. Feel free to ignore the original suggestion.Likely an incorrect or invalid review comment.
| import type { ActionTarget, ImageContentData, InsertImageAction } from '@onlook/models'; | ||
| import { convertToBase64, generateNewFolderPath, getBaseName, getMimeType, isImageFile, stripImageFolderPrefix } from '@onlook/utility'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Import source consolidation looks fine; verify re-exports and type shapes to prevent breakages
Centralizing imports to @onlook/models and @onlook/utility is good. Please verify that:
- @onlook/models re-exports ActionTarget, ImageContentData, InsertImageAction with the same shapes used here.
- @onlook/utility re-exports convertToBase64, generateNewFolderPath, getBaseName, getMimeType, isImageFile, stripImageFolderPrefix.
Run this script to confirm presence and shapes of the re-exports:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking type re-exports in @onlook/models…"
rg -n --no-heading $'export( type)? \\{?\\s*ActionTarget\\b' -A 2 -B 2
rg -n --no-heading $'export( type)? \\{?\\s*ImageContentData\\b' -A 2 -B 2
rg -n --no-heading $'export( type)? \\{?\\s*InsertImageAction\\b' -A 2 -B 2
echo
echo "Enumerating ActionTarget shape(s)…"
rg -n --no-heading $'interface ActionTarget\\b' -A 20 -B 2
rg -n --no-heading $'type ActionTarget\\b' -A 20 -B 2
echo
echo "Checking utility exports in @onlook/utility…"
for sym in convertToBase64 generateNewFolderPath getBaseName getMimeType isImageFile stripImageFolderPrefix; do
echo "Symbol: $sym"
rg -n --no-heading $'export( const| function| type| interface)?\\s+'$sym -A 2 -B 2 || true
rg -n --no-heading $'export \\{[^}]*\\b'$sym'\\b' -A 0 -B 0 || true
echo
doneLength of output: 185
I didn’t see any output because the script aborted on the first missing match. Let’s rerun with a more resilient search across the entire repo (won’t stop on no‐matches) to locate both the re-exports and the actual definitions:
🏁 Script executed:
#!/bin/bash
# Continue even if patterns aren’t found
set -uo pipefail
echo "🔍 Checking @onlook/models for exported types…"
rg -n 'export( type)?[[:space:]]*\{[^}]*\b(ActionTarget|ImageContentData|InsertImageAction)\b[^}]*\}' -A2 -B2 || true
echo
echo "🔍 Locating type definitions in models…"
rg -n 'export (interface|type) (ActionTarget|ImageContentData|InsertImageAction)\b' -A20 -B2 || true
echo
echo "🔍 Checking @onlook/utility for named exports…"
rg -n 'export (const|function|type|interface)[[:space:]]+(convertToBase64|generateNewFolderPath|getBaseName|getMimeType|isImageFile|stripImageFolderPrefix)\b' -A2 -B2 || true
echo
echo "🔍 Checking utility index for grouped exports…"
rg -n 'export\s*\{[^}]*\b(convertToBase64|generateNewFolderPath|getBaseName|getMimeType|isImageFile|stripImageFolderPrefix)\b[^}]*\}' -A0 -B0 || trueLength of output: 5463
Let’s locate and inspect each package’s entry point to confirm the actual re-exports:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "🔍 Locating @onlook/models index file…"
MODELS_INDEX=$(fd index.ts packages/models/src -t f -1) || MODELS_INDEX=""
echo "models index: $MODELS_INDEX"
[ -n "$MODELS_INDEX" ] && sed -n '1,200p' "$MODELS_INDEX" || echo "No index.ts found for models"
echo
echo "🔍 Locating @onlook/utility index file…"
UTILITY_INDEX=$(fd index.ts packages/utility/src -t f -1) || UTILITY_INDEX=""
echo "utility index: $UTILITY_INDEX"
[ -n "$UTILITY_INDEX" ] && sed -n '1,200p' "$UTILITY_INDEX" || echo "No index.ts found for utility"Length of output: 1474
To confirm each package’s published entrypoint and re-export strategy, let’s inspect their package.json files:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== @onlook/models package.json =="
sed -n '1,200p' packages/models/package.json || echo "Cannot read models package.json"
echo
echo "== @onlook/utility package.json =="
sed -n '1,200p' packages/utility/package.json || echo "Cannot read utility package.json"Length of output: 2563
Confirm re-exports in @onlook/models and @onlook/utility
✅ The utility functions you’re importing are all exported through packages/utility/src/index.ts (via export * from './image', ./folder, and ./file'), so convertToBase64, generateNewFolderPath, getBaseName, getMimeType, isImageFile, and stripImageFolderPrefix will resolve correctly.
• packages/models/src/actions/target.ts → export interface ActionTarget
• packages/models/src/actions/action.ts → export interface ImageContentData and export interface InsertImageAction
However, I didn’t find a top-level packages/models/src/index.ts to re-export those interfaces (even though package.json points "." at src/index.ts). Please ensure that your models package’s entrypoint actually exports the action types—either by adding a barrel in src/index.ts or by adjusting the import paths (e.g. from @onlook/models/actions/...).
Tagging as fix required so we don’t accidentally ship broken type imports.
* 'main' of https://github.com/onlook-dev/onlook: (41 commits) feat: fix delete project (onlook-dev#2683) feat: reduce search latency (onlook-dev#2682) feat: revamp projects ux (onlook-dev#2672) Upload fix (onlook-dev#2677) feat: using firecrawl to screenshot (onlook-dev#2665) fix: file upload build error (onlook-dev#2674) feat: new project frames (onlook-dev#2673) feat: self hosting docs (onlook-dev#2671) fix: docs layout + troubleshoot section (onlook-dev#2670) fix(template): suppress hydration mismatch Feat/editor improvements (onlook-dev#2601) fix: make message content selectable (onlook-dev#2664) bugs: transient stream error (onlook-dev#2663) feat: update migration files (onlook-dev#2659) Start from Blank + Import buttons on the home page (onlook-dev#2653) fix: snapshot type conflict and mastra can't view image (onlook-dev#2656) fix: typecheck snapshot (onlook-dev#2655) feat: clean up mastra branch (onlook-dev#2654) feat: mastra no storage (onlook-dev#2650) feat: update ai packages (onlook-dev#2648) ...
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Improves file upload handling in
upload-modal.tsxwith smarter directory selection, binary vs text file distinction, and enhanced error handling.upload-modal.tsx: Smarter directory selection for uploads; images default topublicorassets, non-images use active file's directory.upload-modal.tsx: Distinguishes binary vs text files; useswriteBinaryFilefor binary andwriteFilefor text.upload-modal.tsx: Prevents errors when no file is selected; improved error notifications and resets upload state after attempts.image/index.ts: Consolidates imports from@onlook/utility.upload-modal.tsx: AddsisBinaryFileimport from@onlook/utility.This description was created by
for 27c6baf. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes