From 0571f62a887793bc09ebcfb481c27758a0406516 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Fri, 17 Jul 2026 07:23:46 +0000 Subject: [PATCH] ads: auto-install drops every size above (not smart placement) The Monetize page's "Submit PR to install" installed a single size and tried to place a leaderboard at the top of the page. We can't reliably tell where each size belongs without understanding the target page, so instead inject a unit for EVERY available size (PUBLISHER_FORMAT_IDS) stacked before , sharing one /ad.js loader. Publishers keep or move whichever they want; empty units simply don't render. - lib/github/install-ad.ts: embedBlock stacks all sizes before with a single loader; drop the top-placement / single-format paths - app/api/ads/slots/[id]/install-embed: no per-format param (installs all) - components/ads/slot-manager.tsx: install button says "install all sizes" and sits below the per-size copy area - tests: auto-install emits every size + one loader, all above Co-Authored-By: Claude Opus 4.8 --- app/api/ads/slots/[id]/install-embed/route.ts | 4 - components/ads/slot-manager.tsx | 19 +++- lib/github/install-ad.ts | 94 ++++++++----------- tests/contract/install-ad.test.ts | 15 +-- 4 files changed, 61 insertions(+), 71 deletions(-) diff --git a/app/api/ads/slots/[id]/install-embed/route.ts b/app/api/ads/slots/[id]/install-embed/route.ts index 5aa76c84..65d77da8 100644 --- a/app/api/ads/slots/[id]/install-embed/route.ts +++ b/app/api/ads/slots/[id]/install-embed/route.ts @@ -15,7 +15,6 @@ import { createClient } from "@/lib/supabase/server"; import { serviceClient } from "@/lib/supabase/service"; import { getOrMintInstallationToken } from "@/lib/github/installations"; import { installAdEmbed } from "@/lib/github/install-ad"; -import { AD_FORMAT_IDS } from "@/lib/ads/formats"; export const runtime = "nodejs"; @@ -24,8 +23,6 @@ const bodySchema = z.object({ repo: z.string().min(1).optional(), installation_id: z.number().int().positive().optional(), target_path: z.string().max(500).optional(), - // Which ad size to install. Defaults to the medium rectangle in the installer. - format: z.enum(AD_FORMAT_IDS as [string, ...string[]]).optional(), }); type BoundRepo = { @@ -144,7 +141,6 @@ export async function POST(request: NextRequest, ctx: { params: Promise<{ id: st owner: owner!, repo: repo!, slotId, - format: body.format, targetPath: body.target_path, }); await finalize({ diff --git a/components/ads/slot-manager.tsx b/components/ads/slot-manager.tsx index 01a1ad50..7ddaf38c 100644 --- a/components/ads/slot-manager.tsx +++ b/components/ads/slot-manager.tsx @@ -163,7 +163,7 @@ export function SlotManager({ const res = await fetch(`/api/ads/slots/${slot.id}/install-embed`, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ ...(pick ?? {}), format: fmt }), + body: JSON.stringify(pick ?? {}), }); const json = await res.json(); if (!res.ok) { @@ -269,17 +269,26 @@ export function SlotManager({ 
                   {embed}
-
+
-
)} + {/* Auto-install drops a unit for every size before — we + can't safely guess where each belongs, so the publisher keeps + or moves whichever they want. */} +
+ + + Adds every size above </body>; keep the ones you want. + +
+ {repoChoices && (
Choose a repo:
diff --git a/lib/github/install-ad.ts b/lib/github/install-ad.ts index 9d084f62..671088da 100644 --- a/lib/github/install-ad.ts +++ b/lib/github/install-ad.ts @@ -18,6 +18,7 @@ import { hasDirective, looksLikeCsp, } from "./install-tracker"; +import { PUBLISHER_FORMAT_IDS } from "@/lib/ads/formats"; const AD_ORIGIN = env.siteUrl.replace(/\/$/, ""); const BRANCH_PREFIX = "crawlproof/install-ad-embed"; @@ -67,7 +68,6 @@ export interface InstallAdInput { owner: string; repo: string; slotId: string; - format?: string; rootPath?: string; /** Explicit target file; skips discovery when set. */ targetPath?: string; @@ -84,25 +84,32 @@ export interface InstallAdResult { detail: string; } -const DEFAULT_FORMAT = "banner_300x250"; - -function rawEmbed(slotId: string, format: string): string { - return `
\n `; +function isJsx(path: string): boolean { + return /\.(tsx|jsx)$/.test(path); } -// JSX/TSX layouts: a self-closing div + next/script `; } -function embedForPath(slotId: string, format: string, path: string): string { - return isJsx(path) ? nextEmbed(slotId, format) : rawEmbed(slotId, format); +// We can't reliably tell where each size belongs without understanding the +// page, so the auto-installer drops a unit for every available size stacked +// before , plus a single loader. Publishers move/keep whichever they +// want; empty units simply don't render. +function embedBlock(slotId: string, formats: readonly string[], path: string): string { + return [...formats.map((f) => unitDiv(slotId, f, path)), loaderScript(path)].join("\n"); } // Already installed if this slot's embed OR our /ad.js is present. @@ -127,45 +134,20 @@ function injectBeforeBodyClose(content: string, embed: string, path: string): st const prefix = content.slice(0, idx); const lineStart = prefix.lastIndexOf("\n") + 1; const indent = prefix.slice(lineStart).match(/^\s*/)?.[0] ?? ""; - let updated = `${prefix}${indent} ${embed}\n${indent}${content.slice(idx)}`; - if (isJsx(path) && / depth. + const block = embed + .split("\n") + .map((line) => `${indent} ${line}`) + .join("\n"); + let updated = `${prefix}${block}\n${indent}${content.slice(idx)}`; + if (isJsx(path) && / tag — the "right place" for a -// leaderboard, which reads best across the top of the page rather than jammed -// at the very bottom before . Best-effort: callers fall back to -// injectBeforeBodyClose when there's no tag (e.g. a React fragment). -function injectAfterBodyOpen(content: string, embed: string, path: string): string | null { - const match = content.match(/]*>/i); - if (!match || match.index == null) return null; - const openEnd = match.index + match[0].length; - const lineStart = content.lastIndexOf("\n", match.index) + 1; - const indent = content.slice(lineStart, match.index).match(/^\s*/)?.[0] ?? ""; - let updated = `${content.slice(0, openEnd)}\n${indent} ${embed}${content.slice(openEnd)}`; - if (isJsx(path) && /. -const TOP_PLACED_FORMATS = new Set(["banner_728x90"]); - -// Choose where a format's embed lands. Leaderboards go up top; everything else -// (rectangle, mobile, text link) drops in before . Always falls back to -// the other strategy so a missing / never blocks the install. -function injectEmbed(content: string, embed: string, path: string, format: string): string | null { - if (TOP_PLACED_FORMATS.has(format)) { - return injectAfterBodyOpen(content, embed, path) ?? injectBeforeBodyClose(content, embed, path); - } - return injectBeforeBodyClose(content, embed, path); -} - export async function installAdEmbed(input: InstallAdInput): Promise { - const format = input.format ?? DEFAULT_FORMAT; + const formats = PUBLISHER_FORMAT_IDS; const repoMeta = await getRepo({ token: input.token, owner: input.owner, repo: input.repo }); const base = repoMeta.default_branch; @@ -198,10 +180,10 @@ export async function installAdEmbed(input: InstallAdInput): Promise tag in ${file.path}.` }; } @@ -242,7 +224,7 @@ export async function installAdEmbed(input: InstallAdInput): Promise `\`${f}\``).join(", ")}`, updated - ? `- Injected into \`${file.path}\` before \`\`.` - : `- Ad embed already present in \`${file.path}\`.`, + ? `- Injected all sizes into \`${file.path}\` before \`\`. Move or delete any you don't want — empty units simply don't render.` + : `- Ad units already present in \`${file.path}\`.`, cspBody, "", - "The unit renders inside a sandboxed iframe and never blocks page load. Manage the slot at " + + "Each unit renders inside a sandboxed iframe and never blocks page load. Manage the slot at " + `${AD_ORIGIN}/ads/slots`, ].join("\n"), }); diff --git a/tests/contract/install-ad.test.ts b/tests/contract/install-ad.test.ts index 63a57d2f..af6ee001 100644 --- a/tests/contract/install-ad.test.ts +++ b/tests/contract/install-ad.test.ts @@ -130,10 +130,10 @@ describe("installAdEmbed", () => { expect(written).toContain("next.config.ts"); }); - it("places a leaderboard at the top of the page (after ), not before ", async () => { + it("installs every available size before with a single loader", async () => { github.files.set( "app/layout.tsx", - "export default function RootLayout({ children }) {\n return
{children}
;\n}\n", + "export default function RootLayout({ children }) {\n return {children};\n}\n", ); await installAdEmbed({ @@ -141,16 +141,19 @@ describe("installAdEmbed", () => { owner: "owner", repo: "repo", slotId: "slot-abc", - format: "banner_728x90", }); const write = github.putFile.mock.calls.find((c) => c[0].path === "app/layout.tsx"); expect(write).toBeDefined(); const content = write![0].contentUtf8 as string; - // The embed carries the requested format and lands before
, i.e. right - // after rather than at the very bottom of the page. + // A unit for every publisher size… + expect(content).toContain('data-format="banner_300x250"'); expect(content).toContain('data-format="banner_728x90"'); - expect(content.indexOf("data-cp-ad")).toBeLessThan(content.indexOf("
")); + // …a single shared /ad.js loader for all of them… + expect(content.match(/ad\.js/g)?.length).toBe(1); + // …and everything lands above . + expect(content.indexOf("data-cp-ad")).toBeLessThan(content.indexOf("")); + expect(content.lastIndexOf("data-cp-ad")).toBeLessThan(content.indexOf("")); }); it("no-ops when the embed exists and no CSP needs changes", async () => {