fix: add pathname based refresh to GoogleAdSense #18#26
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR enhances the GoogleAdSense component to reload the AdSense script whenever the Next.js route changes by capturing the current pathname and incorporating it into the Script component’s key. Sequence diagram for AdSense script reload on route changesequenceDiagram
participant NextRouter
participant GoogleAdSense
participant Script
NextRouter->>GoogleAdSense: Route changes (pathname updates)
GoogleAdSense->>Script: Rerender with new key (includes pathname)
Script->>Browser: Loads AdSense script for new page
Class diagram for updated GoogleAdSense componentclassDiagram
class GoogleAdSense {
+GoogleAdSenseProps props
+debug: boolean
+publisherId: string
+pathname: string
+_publisherId: string
+JSX.Element | null
}
GoogleAdSense : usePathname()
GoogleAdSense : Script (key includes pathname)
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Add a
use clientdirective at the top of this file since you’re using theusePathnamehook in a client component. - Don’t forget to import
usePathnamefromnext/navigationto avoid a reference error.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add a `use client` directive at the top of this file since you’re using the `usePathname` hook in a client component.
- Don’t forget to import `usePathname` from `next/navigation` to avoid a reference error.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
WalkthroughAdded Next.js pathname awareness to GoogleAdSense by importing usePathname and incorporating it into the Script element’s key so the ad script remounts on route changes. No public APIs or exports were altered. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Router as Next.js Router
participant GA as GoogleAdSense
participant Script as Next Script (AdSense)
Router->>GA: Render component (initial route)
GA->>Script: Render with key = _publisherId + pathname
Note right of Script: Script mounts for current route
Router-->>GA: Route change (pathname updated)
GA->>Script: Render with new key (_publisherId + new pathname)
Note right of Script: Previous Script unmounts<br/>New Script mounts for new route
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/GoogleAdSense.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (1)
src/GoogleAdSense.tsx (1)
38-39: Implementation looks correct, pending critical fix.The key construction properly includes the pathname to trigger remount on navigation. The explanatory comment is helpful for maintainability. Once the missing
"use client"directive is added and the approach is verified, this implementation should work as intended.
| // ref: https://github.com/btk/nextjs-google-adsense/blob/master/src/components/GoogleAdSense.tsx | ||
| // ref: https://medium.com/frontendweb/how-to-add-google-adsense-in-your-nextjs-89e439f74de3 | ||
|
|
||
| import { usePathname } from "next/navigation"; |
There was a problem hiding this comment.
Critical: Add "use client" directive.
usePathname() is a client-side hook that requires this component to be a Client Component. Without the "use client" directive at the top of the file, this will throw a runtime error in Next.js App Router.
Apply this diff to add the required directive:
+
+'use client';
+
// ref: https://github.com/btk/nextjs-google-adsense/blob/master/src/components/GoogleAdSense.tsx
// ref: https://medium.com/frontendweb/how-to-add-google-adsense-in-your-nextjs-89e439f74de3🤖 Prompt for AI Agents
In src/GoogleAdSense.tsx around line 4, the component uses the client-side hook
usePathname() but the file is missing the "use client" directive; add a single
line containing "use client" as the very first line of the file (before any
imports) so the component is treated as a Client Component and the usePathname()
hook can run without runtime errors.
| debug = false, | ||
| ...props | ||
| }: GoogleAdSenseProps): JSX.Element | null => { | ||
| const pathname = usePathname(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify that remounting the script is the optimal approach.
While this implementation correctly triggers a remount on route changes, consider whether this is the best approach for AdSense. Remounting the script on every navigation may:
- Re-download and re-execute the script unnecessarily
- Cause flickering or layout shifts
- Impact page load performance
Please verify whether Google AdSense recommends this approach for SPAs, or if programmatic ad refresh (e.g., using adsbygoogle.push()) would be more performant.
🌐 Web query:
What is the recommended approach for refreshing Google AdSense ads on route changes in Next.js applications? Should the script be remounted or should ads be refreshed programmatically?
💡 Result:
Short answer: load the ad script once and refresh ad slots programmatically on route changes — don’t repeatedly remount the script tag.
Why: Google’s publisher APIs provide refresh/clear methods (e.g., googletag.pubads().refresh()) or, for classic AdSense ad units, calling (adsbygoogle = window.adsbygoogle || []).push({}) for each unit after navigation. Remounting the global script on every route can cause race conditions and duplicate loads; programmatic refresh follows Google best practices (also avoid refreshing too frequently and only refresh viewable slots). [1][2][3]
References:
- Google Publisher Tag — control ad loading & refresh (best practices, refresh() usage and limits). [1]
- Community guidance / Next.js examples recommending loading the script once and refreshing slots (Stack Overflow). [2][3]
Refresh AdSense slots programmatically instead of remounting the script
Load the ad script once (e.g., in _app.tsx or src/GoogleAdSense.tsx without key={pathname}), then on each route change call (window.adsbygoogle = window.adsbygoogle || []).push({}) or googletag.pubads().refresh() per Google’s best practices. Avoid duplicate script loads, race conditions, and layout shifts.
Summary by Sourcery
Bug Fixes:
Summary by CodeRabbit