Fix complex website preview loading and broken asset resolution#30
Conversation
Co-authored-by: Siraj Chokshi <SirajChokshi@users.noreply.github.com>
Co-authored-by: Siraj Chokshi <SirajChokshi@users.noreply.github.com>
Co-authored-by: Siraj Chokshi <SirajChokshi@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: Siraj Chokshi <SirajChokshi@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: External script append doesn't wait for load
- Replaced the async function with a Promise-based implementation using onload/onerror handlers so the await in loadPageElements actually waits for each external script to load before proceeding.
- ✅ Fixed: Duplicate
getRepositoryRootwith divergent behavior- Removed the duplicate local function from image-fallback.ts and imported the canonical exported version from lang/html.ts, consolidating the logic in one place.
- ✅ Fixed: Property
currentPageUrlis assigned but never read- Removed the unused currentPageUrl property declaration and its assignment in loadHTML since no code ever reads it.
Or push these changes by commenting:
@cursor push 50aaad52cc
Preview (50aaad52cc)
diff --git a/src/utils/image-fallback.ts b/src/utils/image-fallback.ts
--- a/src/utils/image-fallback.ts
+++ b/src/utils/image-fallback.ts
@@ -1,29 +1,7 @@
import { resourceType } from '../types/resources'
+import { getRepositoryRoot } from './lang/html'
import { getResourceType, isValidURL, processUrl } from './url'
-function getRepositoryRoot(rawHtmlUrl: URL): string | undefined {
- const segments = rawHtmlUrl.pathname.split('/').filter(Boolean)
-
- if (
- rawHtmlUrl.hostname === 'raw.githubusercontent.com' &&
- segments.length >= 3
- ) {
- const [owner, repo, branch] = segments
- return `https://${rawHtmlUrl.hostname}/${owner}/${repo}/${branch}/`
- }
-
- const rawMarker = segments.findIndex(
- (segment, idx) => segment === '-' && segments[idx + 1] === 'raw',
- )
- if (rawHtmlUrl.hostname === 'gitlab.com' && rawMarker >= 2) {
- return `https://${rawHtmlUrl.hostname}/${segments
- .slice(0, rawMarker + 3)
- .join('/')}/`
- }
-
- return undefined
-}
-
export function sanitizeRelativePath(
pathParam: string | undefined,
): string | undefined {
diff --git a/src/utils/lang/html.ts b/src/utils/lang/html.ts
--- a/src/utils/lang/html.ts
+++ b/src/utils/lang/html.ts
@@ -126,7 +126,7 @@
return `/${normalized.join('/')}`
}
-function getRepositoryRoot(url: URL): string {
+export function getRepositoryRoot(url: URL): string {
const pathname = normalizePathname(url.pathname)
const segments = pathname.split('/').filter(Boolean)
diff --git a/src/utils/preview.ts b/src/utils/preview.ts
--- a/src/utils/preview.ts
+++ b/src/utils/preview.ts
@@ -19,7 +19,6 @@
// Session
resources: Record<string, string> = {}
- currentPageUrl: URL | undefined
// TODO - might be worth memoizing against the URL for the session
pageData: Partial<HTMLPageData> = {}
@@ -93,7 +92,6 @@
}
const { processedHTML, title } = processHTML(data, url)
- this.currentPageUrl = new URL(url)
this.pageData.title = title ?? url
@@ -262,18 +260,22 @@
this.iframeDocument.document.head.appendChild(tag)
}
- private async appendExternalScriptToHead(
+ private appendExternalScriptToHead(
src: string,
type?: 'module',
): Promise<void> {
- const tag = this.iframeDocument.document.createElement('script')
+ return new Promise<void>((resolve, reject) => {
+ const tag = this.iframeDocument.document.createElement('script')
- if (type) {
- tag.type = type
- }
+ if (type) {
+ tag.type = type
+ }
- tag.src = src
- this.iframeDocument.document.head.appendChild(tag)
+ tag.onload = () => resolve()
+ tag.onerror = () => reject(new Error(`Failed to load script: ${src}`))
+ tag.src = src
+ this.iframeDocument.document.head.appendChild(tag)
+ })
}
/**You can send follow-ups to the cloud agent here.
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: External script failure crashes entire preview rendering
- Wrapped the
await this.appendExternalScriptToHead(source, type)call in a try/catch block that logs a warning and continues, matching the error handling already present in the proxied script branch.
- Wrapped the
Or push these changes by commenting:
@cursor push afdea6ba15
Preview (afdea6ba15)
diff --git a/src/utils/preview.ts b/src/utils/preview.ts
--- a/src/utils/preview.ts
+++ b/src/utils/preview.ts
@@ -195,7 +195,11 @@
continue
}
- await this.appendExternalScriptToHead(source, type)
+ try {
+ await this.appendExternalScriptToHead(source, type)
+ } catch (err) {
+ logger.warn(`Skipping external script after failed load: ${err}`)
+ }
continue
}You can send follow-ups to the cloud agent here.
Co-authored-by: Siraj Chokshi <SirajChokshi@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Non-JavaScript script types lost during deferred execution
- Changed loadPageElements to preserve all original script types (not just 'module') when re-injecting deferred scripts, while filtering out the internal placeholder type to avoid leaking it.
Or push these changes by commenting:
@cursor push 37735f7efe
Preview (37735f7efe)
diff --git a/src/utils/preview.ts b/src/utils/preview.ts
--- a/src/utils/preview.ts
+++ b/src/utils/preview.ts
@@ -174,12 +174,17 @@
for (const script of [...$script]) {
const source =
script.getAttribute(PREVIEW_SCRIPT_SRC_ATTRIBUTE) ?? script.src ?? ''
+ const savedType = script
+ .getAttribute(PREVIEW_SCRIPT_TYPE_ATTRIBUTE)
+ ?.toLowerCase()
+ .trim()
+ const fallbackType = script.getAttribute('type')?.toLowerCase().trim()
const scriptType =
- script
- .getAttribute(PREVIEW_SCRIPT_TYPE_ATTRIBUTE)
- ?.toLowerCase()
- .trim() ?? script.getAttribute('type')?.toLowerCase().trim()
- const type = scriptType === 'module' ? ('module' as const) : undefined
+ savedType ??
+ (fallbackType !== PREVIEW_SCRIPT_PLACEHOLDER_TYPE
+ ? fallbackType
+ : undefined)
+ const type = scriptType || undefined
const inlinePayload = script.textContent ?? ''
script.remove()
@@ -249,7 +254,7 @@
/**
* Inline JS into the preview head.
*/
- private appendScriptToHead(data: string, type?: 'module'): void {
+ private appendScriptToHead(data: string, type?: string): void {
if (!data) {
return
}
@@ -266,7 +271,7 @@
private appendExternalScriptToHead(
src: string,
- type?: 'module',
+ type?: string,
): Promise<void> {
return new Promise<void>((resolve, reject) => {
const tag = this.iframeDocument.document.createElement('script')You can send follow-ups to the cloud agent here.
Co-authored-by: Siraj Chokshi <SirajChokshi@users.noreply.github.com>
Co-authored-by: Siraj Chokshi <SirajChokshi@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Click handler resolves URLs against wrong base
- Replaced manual URL resolution via
new URL(href, iframeDocument.location.href)with$anchor.href(the IDL property), which automatically resolves against the document's<base>tag instead of the iframe'sabout:blanklocation.
- Replaced manual URL resolution via
Or push these changes by commenting:
@cursor push b9c7a51fbd
Preview (b9c7a51fbd)
diff --git a/src/utils/preview.ts b/src/utils/preview.ts
--- a/src/utils/preview.ts
+++ b/src/utils/preview.ts
@@ -115,22 +115,17 @@
return
}
- let resolvedUrl: URL
- try {
- resolvedUrl = new URL(href, this.iframeDocument.location.href)
- } catch {
- return
- }
+ const resolvedUrl = $anchor.href
// Keep external links inside the iframe navigation flow.
- if (!this.isProxySupportedUrl(resolvedUrl.toString())) {
+ if (!this.isProxySupportedUrl(resolvedUrl)) {
return
}
e.preventDefault()
// use Svelte navigation to trigger a re-render from the top of the UI
- goto(`/${encodeURIComponent(resolvedUrl.toString())}`)
+ goto(`/${encodeURIComponent(resolvedUrl)}`)
})
})
}You can send follow-ups to the cloud agent here.
Co-authored-by: Siraj Chokshi <SirajChokshi@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Selectors include
[content]but never process it- Removed
[content]from the querySelectorAll selectors in all three functions sincecontentattributes were never processed by the iteration arrays, making the selector dead code that unnecessarily matched elements like<meta>tags.
- Removed
Or push these changes by commenting:
@cursor push eb86479dd9
Preview (eb86479dd9)
diff --git a/src/utils/lang/html.ts b/src/utils/lang/html.ts
--- a/src/utils/lang/html.ts
+++ b/src/utils/lang/html.ts
@@ -88,7 +88,7 @@
function rewriteRootRelativeAttributes(doc: Document) {
const rootRelativeAttributes = ['src', 'href'] as const
- const $assets = doc.querySelectorAll<HTMLElement>('[src], [href], [content]')
+ const $assets = doc.querySelectorAll<HTMLElement>('[src], [href]')
for (const $asset of $assets) {
for (const attr of rootRelativeAttributes) {
@@ -124,7 +124,7 @@
function rewriteRelativeResourceAttributes(doc: Document, pageUrl: URL) {
const urlAttributes = ['src', 'href'] as const
- const $assets = doc.querySelectorAll<HTMLElement>('[src], [href], [content]')
+ const $assets = doc.querySelectorAll<HTMLElement>('[src], [href]')
for (const $asset of $assets) {
const tagName = $asset.tagName.toLowerCase()
@@ -210,7 +210,7 @@
const repositoryRoot = getRepositoryRoot(rawPageUrl)
const rootRelativeAttributes = ['src', 'href'] as const
- const $assets = doc.querySelectorAll<HTMLElement>('[src], [href], [content]')
+ const $assets = doc.querySelectorAll<HTMLElement>('[src], [href]')
for (const $asset of $assets) {
const tagName = $asset.tagName.toLowerCase()You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit f1c95a7. Configure here.
Co-authored-by: Siraj Chokshi <SirajChokshi@users.noreply.github.com>


Summary
origin/maininto this branch and resolvesrc/utils/lang/html.tsconflicts by combining both intents safely.main’s runtime fetch-resolver support by injecting the resolver script into processed HTML before deferred script handling.tests/html.test.tsfrommain, adapted to synthetic fixture URLs and to assert merged behavior (resolver injection + rewrite semantics).Conflict classification and resolution
html.tswere non-overlapping (PREVIEW_SCRIPT_*constants vsgetFetchResolverScript()helper), so both were kept.processHTML()had conflicting implementations (DOMParser pipeline vs regex/string replacement). Resolved by preserving DOMParser architecture and integrating resolver injection as an additive step.Testing
npm run test -- tests/html.test.ts tests/preview.test.ts tests/image-fallback.test.ts tests/proxy.test.tsnpm run lintnpm run checkWalkthrough