Skip to content

feat(website): SVG optimization bypass and page block observability #1514

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

Open
vibegui wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(website): SVG optimization bypass and page block observability #1514

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
16 changes: 16 additions & 0 deletions website/components/Image.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,25 @@ export const getEarlyHintFromSrcProps = (srcProps: {
return earlyHintParts.join("; ");
};

const isSvg = (src: string) => src?.toLowerCase().endsWith(".svg");

const Image = forwardRef<HTMLImageElement, Props>((props, ref) => {
const { preload, loading = "lazy" } = props;

// SVGs are vector graphics and don't need raster optimization
if (isSvg(props.src)) {
return (
<img
{...props}
data-fresh-disable-lock
preload={undefined}
src={props.src}
loading={loading}
ref={ref}
/>
);
}
Comment on lines +258 to +270

@coderabbitai coderabbitai Bot Jan 18, 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.

⚠️ Potential issue | 🟡 Minor

SVG path ignores the preload prop.

When preload={true} is passed for an SVG image, the preload <Head> link is not rendered since the early return bypasses that logic. This could regress LCP for preloaded SVG hero images.

🔧 Suggested fix to support preload for SVG 
   // SVGs are vector graphics and don't need raster optimization
   if (isSvg(props.src)) {
     return (
-      <img
-        {...props}
-        data-fresh-disable-lock
-        preload={undefined}
-        src={props.src}
-        loading={loading}
-        ref={ref}
-      />
+      <>
+        {preload && (
+          <Head>
+            <link as="image" rel="preload" href={props.src} />
+          </Head>
+        )}
+        <img
+          {...props}
+          data-fresh-disable-lock
+          preload={undefined}
+          src={props.src}
+          loading={loading}
+          ref={ref}
+        />
+      </>
     );
   }
📝 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.

Suggested change
// SVGs are vector graphics and don't need raster optimization
if (isSvg(props.src)) {
return (
<img
{...props}
data-fresh-disable-lock
preload={undefined}
src={props.src}
loading={loading}
ref={ref}
/>
);
}
// SVGs are vector graphics and don't need raster optimization
if (isSvg(props.src)) {
return (
<>
{preload && (
<Head>
<link as="image" rel="preload" href={props.src} />
</Head>
)}
<img
{...props}
data-fresh-disable-lock
preload={undefined}
src={props.src}
loading={loading}
ref={ref}
/>
</>
);
}
🤖 Prompt for AI Agents 
In `@website/components/Image.tsx` around lines 258 - 270, The SVG branch in the
Image component short-circuits before handling the preload prop, so when
isSvg(props.src) is true any preload={true} is ignored; update the SVG handling
in Image.tsx to run the same preload/link-registration logic as non‑SVG images
(i.e., check props.preload and emit the Head preload link or call the existing
preload helper) before returning the <img> for SVGs, or remove the early return
and render the SVG path after executing the preload logic so props.preload is
honored while keeping data-fresh-disable-lock, loading, ref and src usage
unchanged.


const shouldSetEarlyHint = !!props.setEarlyHint && preload;
const srcSet = props.srcSet ??
getSrcSet(
Expand Down
8 changes: 8 additions & 0 deletions website/handlers/fresh.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,14 @@ export default function Fresh(
}
},
);
// Set matched page block name for observability headers
if (isFreshCtx<DecoState>(ctx) && page?.metadata?.component) {
// deno-lint-ignore no-explicit-any
(ctx.state as any).matchedPageBlock = page.metadata.component
.replace(/^site\//, '')
.replace(/\.tsx$/, '');
}

Comment on lines +235 to +242

@coderabbitai coderabbitai Bot Jan 18, 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.

⚠️ Potential issue | 🟡 Minor

Fix formatting to resolve pipeline failure.

The CI is failing due to deno fmt --check. The issues are:

  1. Single quotes should be double quotes (Deno default)
  2. Trailing whitespace on line 242
🔧 Proposed fix 
       // Set matched page block name for observability headers
       if (isFreshCtx<DecoState>(ctx) && page?.metadata?.component) {
         // deno-lint-ignore no-explicit-any
         (ctx.state as any).matchedPageBlock = page.metadata.component
-          .replace(/^site\//, '')
-          .replace(/\.tsx$/, '');
+          .replace(/^site\//, "")
+          .replace(/\.tsx$/, "");
       }
-
📝 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.

Suggested change
// Set matched page block name for observability headers
if (isFreshCtx<DecoState>(ctx) && page?.metadata?.component) {
// deno-lint-ignore no-explicit-any
(ctx.state as any).matchedPageBlock = page.metadata.component
.replace(/^site\//, '')
.replace(/\.tsx$/, '');
}
// Set matched page block name for observability headers
if (isFreshCtx<DecoState>(ctx) && page?.metadata?.component) {
// deno-lint-ignore no-explicit-any
(ctx.state as any).matchedPageBlock = page.metadata.component
.replace(/^site\//, "")
.replace(/\.tsx$/, "");
}
🧰 Tools
🪛 GitHub Actions: ci

[error] 239-242: deno fmt --check reported: Found 1 not formatted file in 2083 files.

🤖 Prompt for AI Agents 
In `@website/handlers/fresh.ts` around lines 235 - 242, The CI fails deno fmt
--check due to quoting and trailing whitespace in the block that sets matched
page block; update the string literals to use double quotes and remove the
trailing whitespace: in the isFreshCtx<DecoState>(ctx) branch where you set
(ctx.state as any).matchedPageBlock from page.metadata.component, change the
regex string delimiters from single to double quotes (e.g., "^site/" and
"\\.tsx$") and ensure there is no trailing whitespace or blank characters after
the closing brace so the file passes deno fmt.

if (asJson !== null) {
didFinish = true;
return Response.json(page, { headers: allowCorsFor(req) });
Expand Down
Loading