Uh oh!
There was an error while loading. Please reload this page.
fix(landing): fix Core Web Vitals regressions across public marketing pages - #5522
Merged
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview On hosted, the root layout no longer always renders Bundle and interaction tweaks: always-visible Reviewed by Cursor Bugbot for commit 87cd4e5. Configure here. |
Contributor
Greptile SummaryThis PR improves Core Web Vitals across the public landing and content pages. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(landing): escape </script> breakout ..." | Re-trigger Greptile |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
… pages - root layout unconditionally rendered next-runtime-env's PublicEnvScript, which calls unstable_noStore() and silently forced every route in the app dynamic - marketing pages never got static/ISR caching despite their own revalidate. Gated it to self-hosted only; hosted now uses a static, build-time equivalent (app/_shell/public-env-script.tsx) - removed real pointer-drag handlers from the hero's decorative workflow animation (was draggable despite being aria-hidden) - disabled dragging/panning on the (currently unmounted) landing-preview ReactFlow canvas so it's static-by-default if it's ever wired in - lazy-mount the Product Demo section's duplicate HeroVisual instance via next/dynamic + IntersectionObserver instead of loading it eagerly below the fold - disabled Next.js Link prefetch on always-in-viewport /signup and /login CTAs (navbar, hero, mobile nav) so their JS isn't fetched on every pageview regardless of whether the visitor clicks - removed `unoptimized` from local blog/integration images (including the priority LCP image on every blog post), letting next/image serve resized AVIF/WebP instead of full-size originals
- revert prefetch={false} on below-fold CTAs (cta.tsx, enterprise.tsx) -
contradicts the prefetch-on-approach rule this PR itself documents
- restore unoptimized on avatarUrl and the MDX body-image renderer, both of
which can legitimately hold external URLs outside next.config.ts's
image remotePatterns allow-list
- simplify handleAnchors to a single block argument now that positions are
static (the second "live position" argument was always identical to the
first after the drag-handler removal)
- extract the near-duplicate IntersectionObserver lazy-mount logic shared
by landing-preview-mount.tsx and product-demo-visual-mount.tsx into a
single apps/sim/app/(landing)/hooks/use-lazy-mount.ts hook
- import next-runtime-env's own exported PUBLIC_ENV_KEY constant instead of
a hardcoded string literal, and match its case-insensitive NEXT_PUBLIC_
filter exactly, removing any drift risk between the two implementations
- drop plain inline comments with no TSDoc home in favor of relying on the
existing TSDoc/CLAUDE.md documentation…-*-page components The blog/library split (#5516) moved the blog post/index/author JSX into shared ContentPostPage/ContentIndexPage/ContentAuthorPage components while this branch was in flight, so the original unoptimized removal (verified local-only ogImage paths for both blog and library content) needs to land on those shared components instead of the old per-route JSX.
waleedlatif1force-pushed
the
worktree-landing-perf-cwv
branch
from
July 9, 2026 00:06
a4f3aaa to
adf6109Compare…d structure doc - add a one-line comment on ContentFrontmatterSchema.ogImage documenting that it's rendered without unoptimized and expects a local path, matching the existing avatarUrl comment convention (a reviewer noted ogImage's schema is technically unconstrained and seo.ts has an http-prefix branch, though all current content is local) - add the new hooks/ folder to the (landing) CLAUDE.md structure diagram and name use-lazy-mount.ts directly in the lazy-mount rule
Greptile P1: a NEXT_PUBLIC_* value containing "</script>" would close the inline script early and could inject markup/script into every hosted page. Escape "<" in the serialized JSON before interpolating it, matching the standard JSON-in-script-tag safeguard.
waleedlatif1
commented
Jul 9, 2026
CollaboratorAuthor
@greptile-apps review |
waleedlatif1
commented
Jul 9, 2026
CollaboratorAuthor
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 87cd4e5. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
6 tasks
waleedlatif1 added a commit
that referenced
this pull request
Jul 9, 2026
…nstead (#5528) PR #5522 removed `unoptimized` from local blog/library/integration cover images, expecting next/image's runtime optimizer to serve resized AVIF/WebP. On staging, images broke entirely on /blog - the runtime optimizer is failing there (root cause still under investigation: _next/image requests aren't captured by the app's structured logger, so the underlying error wasn't visible in application logs). Reverting `unoptimized` immediately restores working images. In its place, this compresses the actual source files (mozjpeg quality 82 for JPEGs, palette PNG for PNGs) at their EXACT existing pixel dimensions - verified programmatically per-file (dimension mismatch aborts the write) and spot-checked visually. This gets the same bandwidth/LCP win the runtime optimizer was meant to provide, without depending on it: - 13 blog/library cover images + 3 author avatars + 1 brand logo - ~1.72MB -> ~1.02MB combined (~41% smaller), zero resolution change
waleedlatif1 added a commit
that referenced
this pull request
Jul 12, 2026
* fix(landing): repair Lighthouse-flagged CWV audits on production Empirically verified against a live full Lighthouse run of www.sim.ai (production, pre-fix) plus a local build of the exact deployed commit with source maps temporarily enabled for root-causing. Distinguished genuinely failing audits from passing ones already misread as broken. - fetchPriority missing on every LCP hero image: `priority` generates a preload <link> but Next does not auto-add fetchpriority=high to it - confirmed via raw deployed HTML diff. Added explicit fetchPriority='high' to all 5 priority Image usages (hero, enterprise, blog/library post + index cards). - valid-source-maps failing: production ships no source maps at all (productionBrowserSourceMaps defaults false). Enabled it - safe here since this repo's frontend is already fully open source, so no incremental exposure versus Next's default. - image-delivery-insight (55.8KB wasted): feature-integrate-ui.png's `sizes` hint was a flat 1050px regardless of viewport, so mobile fetched the 1920w variant for a ~423px real render. Replaced with a responsive sizes expression derived from the sibling backdrop image's own (already correct) hint, scaled by the callout's documented 125% overhang. - cache-insight (best-fixable portion): _next/static/* filenames are content-hashed and immutable per deploy, but shared one cache rule with unhashed /public assets, capping both at 1-day max-age. Split into two rules - hashed assets now get 1-year immutable, unhashed assets keep the shorter revalidating TTL. Verified via a real build + server that both paths now return the correct distinct header. Investigated and NOT changed (documented, not assumed): - legacy-javascript-insight (14KB): traced via sourcemap to next/dist/build/polyfills/polyfill-module.js - Next's own built-in polyfill bundle, not our code or a dependency, and not exposed via any next.config.ts option. No browserslist misconfiguration on our end (none exists; Next already defaults to its modern target). - forced-reflow-insight: even with source maps present locally, the dominant cost (335-417ms) stayed [unattributed] by Chrome's own profiler, and the small attributed slice was non-deterministic between our own chunk and a third-party script (HubSpot analytics) across runs - not a confident single root cause worth a targeted fix. - render-blocking-insight / network-dependency-tree / bf-cache: bf-cache's actual failure reason is Cache-Control: no-store on the main document - the exact root cause already fixed on staging (PR #5522/#5528, the PublicEnvScript/unstable_noStore fix) but not yet promoted to main/prod. Resolves once that ships, not additional work here. * fix(landing): convert mothership cover from PNG to JPEG (/blog LCP 6.6s -> 2.8s) Ran a full Lighthouse sweep across every public page as requested. /blog scored 73 (LCP 6.6s) while every other page scored 95+ - reproduced consistently across 3 runs, not noise. Traced via lcp-breakdown-insight: the LCP image (mothership/cover.png, 241KB even after the earlier palette compression pass) took 6+ seconds to download on simulated mobile throttling, well beyond what its size should cost. PNG is a poor fit for this illustration's subtle gradients versus JPEG's lossy compression. Verified empirically before converting: same 1920x1080 resolution, visually identical (spot-checked), 241KB -> 65KB (73% smaller). No other cover in the content set uses PNG and benefits the same way (checked copilot/cover.png, the only other PNG cover - already optimal at 64KB, converting it yielded no improvement, left unchanged). Verified fix: /blog score 73->93, LCP 6.6s->2.8s, reproduced across 3 runs. * fix(landing): correct mobile sizes tier, drop non-functional cache rule - integrations-callout: account for FeatureCard's max-lg:grid-cols-1 mobile stack in the sizes hint, verified against Lighthouse's measured mobile render width. - next.config: remove a custom _next/static cache-control rule that never actually fired (confirmed via header-marker test) - Next's own built-in default already applies the correct immutable 1yr cache to that path. * fix(landing): correct sizes underestimate + fix dead .map header rule - integrations-callout: derive sizes from the section's actual grid math (fixed 386px copy column, 40px gap, section gutters) instead of an approximated vw fraction. Verified against a static reproduction of the layout rendered at each Tailwind breakpoint - the old 110vw mobile tier underestimated real render width by ~3% right at the 1023px stack boundary, which could cause the browser to pick a too-small srcset candidate and upscale. - next.config: the .map header rule's trailing `$` was read as a literal character by Next's path-to-regexp source matcher, not a regex anchor, so the rule never matched a real .map URL (confirmed via routes-manifest regex + a live header check). Removed the dead anchor and added a bounded Cache-Control so a future decision to stop shipping source maps isn't undermined by a 1yr immutable cache on already-fetched maps. * fix(llms): serve well-formed llms.txt, remove Mothership + dead static files Both the marketing site and docs site's llms.txt validator errors ("does not appear to contain any links") traced to the same root cause: a static public/llms.txt shadowed a better-written, already-existing dynamic app/llms.txt route, and every "link" in the static files (and in the docs app's auto-generated route) was bare `label: url` text, not Markdown link syntax - so a strict Markdown-link parser found zero matches even though URLs were visibly present. - apps/sim: delete public/llms.txt (dead code, shadowing the properly Markdown-linked app/llms.txt route.ts, confirmed via production headers showing the static file was what actually served). Fix llms-full.txt's Links/Support/Legal sections to use [label](url) syntax, correct a stale "Next.js 15" reference, and replace "Mothership" with "Chat" per the constitution's language rules. - apps/docs: same shadowing issue - delete the orphaned public/llms.txt (also still said "Mothership"). Fix the auto-generated per-page link list in app/llms.txt/route.ts to emit [title](url) instead of "title: url" for every documentation page. * fix(llms): actually include the route.ts fixes from the prior commit The prior commit (3b2d35c) only staged the two deleted public/llms.txt files - these two modified route.ts files (the Mothership/link-format fixes they were meant to accompany) were left unstaged. No new changes, just completing that commit's intent.
6 tasks
waleedlatif1 added a commit
that referenced
this pull request
Aug 3, 2026
…6214) The hosted `<PublicEnvScript>` rendered a plain `<script>`, which lands at the end of `<head>` — after the ~40 `<script async>` chunk tags Next emits at the top of the document. An async script runs as soon as its fetch resolves, so on a warm cache a Next chunk could execute (and hydration begin) before the parser reached the env tag, leaving `window.__ENV` undefined for the first render. That surfaced as "Something went wrong" on the workflow page, since `getBaseUrl()` throws during the deploy modal's render, and as the socket falling back to the page origin instead of NEXT_PUBLIC_SOCKET_URL. Regressed in #5522, which replaced next-runtime-env's PublicEnvScript (to avoid its unstable_noStore forcing dynamic rendering) with a static equivalent that dropped the beforeInteractive strategy. - render the library's own `<EnvScript>`, which defaults to beforeInteractive and does not call unstable_noStore — hosted and self-hosted now share one implementation and one loading strategy - drop the hand-rolled serialization and `<` escaping; Next's beforeInteractive path already runs the payload through htmlEscapeJsonString, which escapes `& > < U+2028 U+2029` - fall back to the browser origin in getBaseUrl() rather than throwing, so a missing injected env can never tear down a page through the error boundary; server-side callers still fail loudly - read NEXT_PUBLIC_EMAIL_PASSWORD_SIGNUP_ENABLED via getEnv() in the SSO form, matching login/signup/auth-modal — `env.X` returns the build-time placeholder, not the runtime value
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
next-runtime-env'sPublicEnvScript, which callsunstable_noStore()and silently forced every route in the app into dynamic rendering - marketing pages never got static/ISR caching despite their ownrevalidate. Confirmed viabun run build's route table (ƒ→○/●) and prod'sCache-Control: private, no-storeheader. Gated it to self-hosted only; hosted now uses a static, build-time equivalent (app/_shell/public-env-script.tsx) that populates the samewindow.__ENVglobal.aria-hidden.HeroVisualinstance vianext/dynamic+IntersectionObserverinstead of loading it eagerly below the fold.<Link>prefetch on always-in-viewport/signupand/loginCTAs (navbar, hero, mobile nav) so their JS isn't fetched on every pageview regardless of whether the visitor clicks.unoptimizedfrom local blog/integration images (including the priority LCP image on every blog post), lettingnext/imageserve resized AVIF/WebP instead of full-size originals.(landing)/CLAUDE.md(decorative-canvas-must-be-static, lazy-mount pattern, prefetch={false} convention, root-layout dynamic-rendering gotcha).Type of Change
Testing
bun run build --filter=simwithNEXT_PUBLIC_APP_URL=https://sim.ai(isHosted=true):/,/enterprise,/workflows,/solutions/*,/demo,/blog/tags,/blog/[slug],/models/[provider],/models/[provider]/[model],/integrations/[slug]all flip fromƒto○/●.window['__ENV']renders correctly in the static HTML output with real values.next startlocally: response header changes fromCache-Control: private, no-cache, no-storetoCache-Control: no-cache, must-revalidate(ISR-eligible) on/.tsc --noEmitclean (no new errors),bun run lintclean.Checklist