Conversation
|
@sushilbang is attempting to deploy a commit to the sz47's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe template system was migrated from local JSON files to ZIP-based templates hosted externally. A ZipExtractor and loader now fetch ZIPs, extract files, build asset blob URLs, resolve template asset references, and apply the resolved template to the editor with loading indicators and error handling. Changes
Sequence DiagramsequenceDiagram
participant User
participant Editor as Editor UI
participant Fetch as HTTP Fetch
participant Zip as ZipExtractor
participant Resolver as Asset Resolver
participant EditorCore as Editor Core
participant Storage as IndexedDB
User->>Editor: Select template
Editor->>Editor: Show loading indicator
Editor->>Fetch: GET template.zip (TEMPLATES_BASE_URL/.../templateId.zip)
Fetch-->>Editor: ZIP arrayBuffer
Editor->>Zip: new ZipExtractor(arrayBuffer)
Zip->>Zip: parse EOCD & central directory
Editor->>Zip: extractAll()
Zip-->>Editor: Map<filename, Blob>
Editor->>Editor: Parse template.json from files
Editor->>Editor: Build assetMap (filename -> blob URL)
Editor->>Resolver: resolveTemplateAssets(template, assetMap)
Resolver-->>Editor: resolved template (assets replaced by blob URLs)
Editor->>EditorCore: wisk.editor.useTemplate(resolved template)
EditorCore->>Storage: Persist referenced assets as needed
Storage-->>EditorCore: Assets stored
EditorCore-->>Editor: Template applied
Editor->>Editor: Hide loading indicator
Editor-->>User: Template loaded / error toast on failure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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
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: 1
🤖 Fix all issues with AI agents
In `@js/plugins/code/image-element.js`:
- Around line 1120-1142: In storeBlobUrl, sanitize the file extension derived
from blob.type (handle complex types like image/svg+xml by stripping anything
after '+' or ';' so "svg+xml" becomes "svg") and add a safe fallback extension
(e.g., "png") if parsing fails; also stop failing silently on errors by
surfacing the failure to callers—either rethrow the caught error or return a
rejected Promise and ensure a clear error message is logged (include the blobUrl
and error) so callers of storeBlobUrl can display or handle the failure.
🧹 Nitpick comments (3)
js/utils.js (2)
311-325: Memory leak: Blob URLs are created but never revoked.
URL.createObjectURLcreates blob URLs that persist until explicitly revoked withURL.revokeObjectURLor the page is unloaded. For templates with many assets, this can lead to memory accumulation.Consider:
- Documenting that callers are responsible for cleanup
- Returning a cleanup function that revokes all blob URLs
- Adding a
dispose()method or returningrevokeAllcallback💡 Suggested enhancement
+ // Collect all blob URLs for potential cleanup + const blobUrls = []; + // Create asset map with blob URLs for all assets const assetMap = {}; for (const [path, blob] of files) { // Normalize path separators (Windows uses backslash in ZIP) const normalizedPath = path.replace(/\\/g, '/'); if (normalizedPath.startsWith('assets/') || normalizedPath.includes('/assets/')) { // Create blob URL for asset const blobUrl = URL.createObjectURL(blob); + blobUrls.push(blobUrl); // Store with multiple path formats for matching assetMap[`./${normalizedPath}`] = blobUrl; assetMap[normalizedPath] = blobUrl; assetMap[`./${path}`] = blobUrl; assetMap[path] = blobUrl; } } // ... rest of function ... return { template, assetMap, previews, files, + revokeAll: () => blobUrls.forEach(url => URL.revokeObjectURL(url)), };
166-224: Consider documenting browser requirements for DecompressionStream.The
DecompressionStreamAPI with'deflate-raw'is supported in modern browsers (~95% global coverage) but unavailable in Firefox < 113 and Safari < 16.4. If supporting older browsers is a requirement, add a fallback decompression library or document the minimum browser versions needed for this ZIP extraction functionality.js/elements/home-element.js (1)
3-4: Duplicate constant:TEMPLATES_BASE_URLis defined in multiple files.This constant is duplicated in
js/elements/template-dialog.js(line 5) andjs/editor.js(line 593). Consider extracting to a shared module or configuration to maintain a single source of truth.#!/bin/bash # Find all occurrences of TEMPLATES_BASE_URL definition rg -n "TEMPLATES_BASE_URL\s*=" --type js
| async storeBlobUrl(blobUrl) { | ||
| try { | ||
| // Fetch the blob | ||
| const response = await fetch(blobUrl); | ||
| const blob = await response.blob(); | ||
|
|
||
| // Generate a unique filename | ||
| const extension = blob.type.split('/')[1] || 'png'; | ||
| const localUrl = 'template-image-' + Date.now() + '.' + extension; | ||
|
|
||
| // Save to IndexedDB | ||
| await wisk.db.setAsset(localUrl, blob); | ||
|
|
||
| // Set the new URL and update | ||
| this.imageUrl = localUrl; | ||
| this.updateImage(); | ||
| this.sendUpdates(); | ||
|
|
||
| return localUrl; | ||
| } catch (error) { | ||
| console.error('[Image Element] Error storing blob URL:', error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Edge case: MIME type parsing may produce invalid extensions for complex types.
For MIME types like image/svg+xml, blob.type.split('/')[1] produces svg+xml instead of svg. Consider sanitizing the extension.
Also, the function silently fails on error without notifying the user, which could lead to confusion when template images don't appear.
🔧 Proposed fix
async storeBlobUrl(blobUrl) {
try {
// Fetch the blob
const response = await fetch(blobUrl);
const blob = await response.blob();
// Generate a unique filename
- const extension = blob.type.split('/')[1] || 'png';
+ let extension = blob.type.split('/')[1] || 'png';
+ // Handle complex MIME types like image/svg+xml
+ extension = extension.split('+')[0];
const localUrl = 'template-image-' + Date.now() + '.' + extension;
// Save to IndexedDB
await wisk.db.setAsset(localUrl, blob);
// Set the new URL and update
this.imageUrl = localUrl;
this.updateImage();
this.sendUpdates();
return localUrl;
} catch (error) {
console.error('[Image Element] Error storing blob URL:', error);
+ wisk.utils.showToast('Failed to load template image', 3000);
}
}🤖 Prompt for AI Agents
In `@js/plugins/code/image-element.js` around lines 1120 - 1142, In storeBlobUrl,
sanitize the file extension derived from blob.type (handle complex types like
image/svg+xml by stripping anything after '+' or ';' so "svg+xml" becomes "svg")
and add a safe fallback extension (e.g., "png") if parsing fails; also stop
failing silently on errors by surfacing the failure to callers—either rethrow
the caught error or return a rejected Promise and ensure a clear error message
is logged (include the blobUrl and error) so callers of storeBlobUrl can display
or handle the failure.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@js/editor.js`:
- Around line 600-606: The code calls wisk.editor.useTemplate inside a
setTimeout without awaiting it, causing unhandled rejections and hiding the
loader too early; update this block to await
wisk.editor.useTemplate(resolvedTemplate) (remove the unnecessary setTimeout),
wrap the await in try/catch to surface/log errors, and call
wisk.utils.hideLoading() in a finally so the loader is only hidden after
useTemplate finishes or fails; keep wisk.utils.resolveTemplateAssets as-is and
ensure any caught error is logged or rethrown as appropriate.
In `@js/elements/template-dialog.js`:
- Around line 372-387: In useTemplate(), await the asynchronous editor call so
errors are caught: change the call to await
wisk.editor.useTemplate(resolvedTemplate) and ensure cleanup runs after that by
moving wisk.utils.hideLoading() and this.hide() into a finally block (or call
them after the awaited call inside the try) so the loading UI is correctly
handled whether useTemplate succeeds or throws; references: useTemplate(),
wisk.utils.showLoading, wisk.utils.loadTemplateFromZip,
wisk.utils.resolveTemplateAssets, wisk.editor.useTemplate,
wisk.utils.hideLoading, this.hide().
🧹 Nitpick comments (2)
js/editor.js (1)
592-595: Avoid duplicating the templates base URL.
TEMPLATES_BASE_URLis already centralized elsewhere; re-defining it here risks drift. Consider reusing a shared constant (e.g., viawisk.utilsor a single config module).js/elements/template-dialog.js (1)
323-327: Handle non-OK responses for the external templates fetch.
The new external URL increases the chance of non-200s; consider checkingresponse.okand throwing a descriptive error beforeresponse.json()to improve diagnostics and user feedback.
| .then(({ template, assetMap }) => { | ||
| // Resolve asset references in template | ||
| const resolvedTemplate = wisk.utils.resolveTemplateAssets(template, assetMap); | ||
| setTimeout(() => { | ||
| wisk.editor.useTemplate(data); | ||
| wisk.editor.useTemplate(resolvedTemplate); | ||
| wisk.utils.hideLoading(); | ||
| }, 0); |
There was a problem hiding this comment.
Await wisk.editor.useTemplate to avoid premature loader hide and unhandled rejections.
useTemplate is async; without await, failures won’t be caught and the loading UI hides before template application completes.
🛠️ Suggested fix
- wisk.utils
- .loadTemplateFromZip(zipUrl)
- .then(({ template, assetMap }) => {
+ wisk.utils
+ .loadTemplateFromZip(zipUrl)
+ .then(({ template, assetMap }) => {
// Resolve asset references in template
const resolvedTemplate = wisk.utils.resolveTemplateAssets(template, assetMap);
- setTimeout(() => {
- wisk.editor.useTemplate(resolvedTemplate);
- wisk.utils.hideLoading();
- }, 0);
+ setTimeout(async () => {
+ try {
+ await wisk.editor.useTemplate(resolvedTemplate);
+ } finally {
+ wisk.utils.hideLoading();
+ }
+ }, 0);
})🤖 Prompt for AI Agents
In `@js/editor.js` around lines 600 - 606, The code calls wisk.editor.useTemplate
inside a setTimeout without awaiting it, causing unhandled rejections and hiding
the loader too early; update this block to await
wisk.editor.useTemplate(resolvedTemplate) (remove the unnecessary setTimeout),
wrap the await in try/catch to surface/log errors, and call
wisk.utils.hideLoading() in a finally so the loader is only hidden after
useTemplate finishes or fails; keep wisk.utils.resolveTemplateAssets as-is and
ensure any caught error is logged or rethrown as appropriate.
| async useTemplate() { | ||
| // get template json from /js/templates/storage/${this.selectedTemplate.path}.json | ||
| // run wisk.editor.useTemplate(template) | ||
| const response = await fetch(`/js/templates/storage/${this.selectedTemplate.path}.json`); | ||
| const data = await response.json(); | ||
| wisk.editor.useTemplate(data); | ||
| this.hide(); | ||
| try { | ||
| // Show loading indicator | ||
| wisk.utils.showLoading('Loading template...'); | ||
|
|
||
| // Load template from ZIP file | ||
| const zipUrl = `${TEMPLATES_BASE_URL}/templates/${this.selectedTemplate.id}/data.zip`; | ||
| const { template, assetMap } = await wisk.utils.loadTemplateFromZip(zipUrl); | ||
|
|
||
| // Resolve asset references in template | ||
| const resolvedTemplate = wisk.utils.resolveTemplateAssets(template, assetMap); | ||
| // Apply the template | ||
| wisk.editor.useTemplate(resolvedTemplate); | ||
|
|
||
| wisk.utils.hideLoading(); | ||
| this.hide(); |
There was a problem hiding this comment.
Await wisk.editor.useTemplate inside the try block.
Without await, failures won’t be caught and the loading UI can disappear before the template is actually applied.
🛠️ Suggested fix
- // Apply the template
- wisk.editor.useTemplate(resolvedTemplate);
-
- wisk.utils.hideLoading();
- this.hide();
+ // Apply the template
+ await wisk.editor.useTemplate(resolvedTemplate);
+
+ wisk.utils.hideLoading();
+ this.hide();📝 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.
| async useTemplate() { | |
| // get template json from /js/templates/storage/${this.selectedTemplate.path}.json | |
| // run wisk.editor.useTemplate(template) | |
| const response = await fetch(`/js/templates/storage/${this.selectedTemplate.path}.json`); | |
| const data = await response.json(); | |
| wisk.editor.useTemplate(data); | |
| this.hide(); | |
| try { | |
| // Show loading indicator | |
| wisk.utils.showLoading('Loading template...'); | |
| // Load template from ZIP file | |
| const zipUrl = `${TEMPLATES_BASE_URL}/templates/${this.selectedTemplate.id}/data.zip`; | |
| const { template, assetMap } = await wisk.utils.loadTemplateFromZip(zipUrl); | |
| // Resolve asset references in template | |
| const resolvedTemplate = wisk.utils.resolveTemplateAssets(template, assetMap); | |
| // Apply the template | |
| wisk.editor.useTemplate(resolvedTemplate); | |
| wisk.utils.hideLoading(); | |
| this.hide(); | |
| async useTemplate() { | |
| try { | |
| // Show loading indicator | |
| wisk.utils.showLoading('Loading template...'); | |
| // Load template from ZIP file | |
| const zipUrl = `${TEMPLATES_BASE_URL}/templates/${this.selectedTemplate.id}/data.zip`; | |
| const { template, assetMap } = await wisk.utils.loadTemplateFromZip(zipUrl); | |
| // Resolve asset references in template | |
| const resolvedTemplate = wisk.utils.resolveTemplateAssets(template, assetMap); | |
| // Apply the template | |
| await wisk.editor.useTemplate(resolvedTemplate); | |
| wisk.utils.hideLoading(); | |
| this.hide(); |
🤖 Prompt for AI Agents
In `@js/elements/template-dialog.js` around lines 372 - 387, In useTemplate(),
await the asynchronous editor call so errors are caught: change the call to
await wisk.editor.useTemplate(resolvedTemplate) and ensure cleanup runs after
that by moving wisk.utils.hideLoading() and this.hide() into a finally block (or
call them after the awaited call inside the try) so the loading UI is correctly
handled whether useTemplate succeeds or throws; references: useTemplate(),
wisk.utils.showLoading, wisk.utils.loadTemplateFromZip,
wisk.utils.resolveTemplateAssets, wisk.editor.useTemplate,
wisk.utils.hideLoading, this.hide().
1 participant
Templates will now be fetched from sohzm.github.io/wisk-templates as ZIP files containing both template data and bundled assets.
Changes
Template ZIP Structure
template-name.zip
├── template.json
└── assets/
└── images
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.