Skip to content

fix(server): favicon resolution no longer pins the event loop #5531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vbb-it wants to merge 1 commit into from
+73 −3
Closed

fix(server): favicon resolution no longer pins the event loop #5531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions apps/server/src/project/ProjectFaviconResolver.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,73 @@ it.layer(TestLayer)("ProjectFaviconResolverLive", (it) => {
}),
);

it.effect("resolves icon hrefs from object-literal route metadata", () =>
Effect.gen(function* () {
const resolver = yield* ProjectFaviconResolver.ProjectFaviconResolver;
const cwd = yield* makeTempDir;
yield* writeTextFile(
cwd,
"src/routes/__root.tsx",
`export const Route = createRootRoute({
head: () => ({
links: [
{ rel: "stylesheet", href: "/app.css" },
{ rel: "icon", href: "/brand/logo.svg" },
],
}),
});`,
);
yield* writeTextFile(cwd, "public/brand/logo.svg", "<svg>brand</svg>");

const resolved = yield* resolver.resolvePath(cwd);

expect(resolved).not.toBeNull();
expect(resolved).toContain("public/brand/logo.svg");
}),
);

it.effect("resolves object-literal icon metadata when href precedes rel", () =>
Effect.gen(function* () {
const resolver = yield* ProjectFaviconResolver.ProjectFaviconResolver;
const cwd = yield* makeTempDir;
yield* writeTextFile(
cwd,
"src/root.tsx",
`const links = [{ href: "/brand/logo.svg", rel: "shortcut icon" }];`,
);
yield* writeTextFile(cwd, "public/brand/logo.svg", "<svg>brand</svg>");

const resolved = yield* resolver.resolvePath(cwd);

expect(resolved).not.toBeNull();
expect(resolved).toContain("public/brand/logo.svg");
}),
);

// A large icon source with no icon metadata used to pin the server's event loop for
// minutes: the object pattern was unanchored, so it restarted at every offset and
// rescanned forward from each one. Anchoring keeps this proportional to file size.
it.effect("scans large icon sources without an icon in reasonable time", () =>
Effect.gen(function* () {
const resolver = yield* ProjectFaviconResolver.ProjectFaviconResolver;
const cwd = yield* makeTempDir;
// Mirrors a generated single-file build: large, brace-sparse, and no icon metadata.
const filler = `<p>${"pokopia companion guide ".repeat(24)}</p>\n`;
yield* writeTextFile(
cwd,
"index.html",
`<!doctype html><html><head><title>guide</title></head><body>\n${filler.repeat(1200)}</body></html>`,
);

const startedAt = performance.now();
const resolved = yield* resolver.resolvePath(cwd);
const elapsedMs = performance.now() - startedAt;

expect(resolved).toBeNull();
expect(elapsedMs).toBeLessThan(5_000);
}),
);

it.effect("returns null when no icon is present", () =>
Effect.gen(function* () {
const resolver = yield* ProjectFaviconResolver.ProjectFaviconResolver;
Expand Down
9 changes: 6 additions & 3 deletions apps/server/src/project/ProjectFaviconResolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,12 @@ const ICON_SOURCE_FILES = [
] as const;

// Matches <link ...> tags or object-like icon metadata where rel/href can appear in any order.
// Both patterns stay anchored on a literal opening delimiter (`<link` / `{`) so the scan starts
// only at real candidates and each attempt is bounded by the enclosing tag or object.
const LINK_ICON_HTML_RE =
/<link\b(?=[^>]*\brel=["'](?:icon|shortcut icon)["'])(?=[^>]*\bhref=["']([^"'?]+))[^>]*>/i;
const LINK_ICON_OBJ_RE =
/(?=[^}]*\brel\s*:\s*["'](?:icon|shortcut icon)["'])(?=[^}]*\bhref\s*:\s*["']([^"'?]+))[^}]*/i;
const LINK_ICON_OBJ_RE = /\{[^{}]*\brel\s*:\s*["'](?:icon|shortcut icon)["'][^{}]*\}/i;

@macroscopeapp macroscopeapp Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium project/ProjectFaviconResolver.ts:62

LINK_ICON_OBJ_RE uses [^{}]* to bound the match, so any icon metadata object containing nested braces fails to match — for example { attributes: {}, rel: "icon", href: "/favicon.svg" } returns null from extractIconHref instead of /favicon.svg. The prior lookahead-based pattern could scan past inner braces; the new anchored pattern cannot. Consider using a pattern that tolerates nested braces, or strip/normalize brace depth before matching.

🤖 Copy this AI Prompt to have your agent fix this: 
In file @apps/server/src/project/ProjectFaviconResolver.ts around line 62:

`LINK_ICON_OBJ_RE` uses `[^{}]*` to bound the match, so any icon metadata object containing nested braces fails to match — for example `{ attributes: {}, rel: "icon", href: "/favicon.svg" }` returns `null` from `extractIconHref` instead of `/favicon.svg`. The prior lookahead-based pattern could scan past inner braces; the new anchored pattern cannot. Consider using a pattern that tolerates nested braces, or strip/normalize brace depth before matching.

@macroscopeapp macroscopeapp Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium project/ProjectFaviconResolver.ts:62

LINK_ICON_OBJ_RE now matches the first {...} object containing rel: "icon" even when that object has no href, so extractIconHref never examines later valid declarations. A source like [{ rel: "icon" }, { rel: "icon", href: "/favicon.svg" }] returns null instead of /favicon.svg. The old regex required both rel and href in the same candidate. Consider restoring the href lookahead inside LINK_ICON_OBJ_RE so non-href objects are skipped.

Suggested change
const LINK_ICON_OBJ_RE = /\{[^{}]*\brel\s*:\s*["'](?:icon|shortcut icon)["'][^{}]*\}/i;
const LINK_ICON_OBJ_RE = /\{(?=[^{}]*\brel\s*:\s*["'](?:icon|shortcut icon)["'])(?=[^{}]*\bhref\s*:\s*["']([^"'?]+))[^{}]*\}/i;
🤖 Copy this AI Prompt to have your agent fix this: 
In file @apps/server/src/project/ProjectFaviconResolver.ts around line 62:

`LINK_ICON_OBJ_RE` now matches the first `{...}` object containing `rel: "icon"` even when that object has no `href`, so `extractIconHref` never examines later valid declarations. A source like `[{ rel: "icon" }, { rel: "icon", href: "/favicon.svg" }]` returns `null` instead of `/favicon.svg`. The old regex required both `rel` and `href` in the same candidate. Consider restoring the `href` lookahead inside `LINK_ICON_OBJ_RE` so non-`href` objects are skipped.

const ICON_HREF_RE = /\bhref\s*:\s*["']([^"'?]+)/i;

export class ProjectFaviconResolutionError extends Schema.TaggedErrorClass<ProjectFaviconResolutionError>()(
"ProjectFaviconResolutionError",
Expand Down Expand Up @@ -99,7 +101,8 @@ function extractIconHref(source: string): string | null {
const htmlMatch = source.match(LINK_ICON_HTML_RE);
if (htmlMatch?.[1]) return htmlMatch[1];
const objMatch = source.match(LINK_ICON_OBJ_RE);
if (objMatch?.[1]) return objMatch[1];
const objHref = objMatch?.[0].match(ICON_HREF_RE);
if (objHref?.[1]) return objHref[1];

@cursor cursor Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skips later valid icon objects

Medium Severity

LINK_ICON_OBJ_RE now matches any brace group with rel: "icon" and no longer requires a quoted href in that same object. extractIconHref then only inspects the first match, so if that object has no extractable href (missing, backtick template, or variable), a later valid icon object is never considered and resolution returns null.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4b77158. Configure here.

return null;
}

Expand Down
Loading