Fix: Hydration warning on local development #4523
Open
+97
−64
Conversation
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
sauhardh
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg and
sambhavgupta0705
as code owners
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
We require all PRs to follow Conventional Commits specification. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
WalkthroughThe PR resolves Next.js hydration errors by replacing inline Changes
Sequence DiagramsequenceDiagram
participant Browser
participant OldFlow as Old: Static Script Tag
participant NewFlow as New: Next.js Script Component
participant ExternalScript as External JS
rect rgb(220, 240, 220)
Note over OldFlow: Hydration Mismatch Risk
Browser->>OldFlow: Initial Page Load
OldFlow->>ExternalScript: Load immediately (timing unclear)
ExternalScript-->>Browser: Script Applied
Note over Browser: ⚠️ SSR/Client mismatch possible
end
rect rgb(240, 220, 220)
Note over NewFlow: Optimized Loading
Browser->>NewFlow: Initial Page Load
Note over NewFlow: Defer to afterInteractive phase
Browser->>NewFlow: Page interactive + hydration complete
NewFlow->>ExternalScript: Load after interaction
ExternalScript-->>Browser: Script Applied
Note over Browser: ✓ Safe hydration
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4523--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
next-env.d.ts(1 hunks)pages/_app.tsx(2 hunks)pages/_document.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: anshgoyalevil
Repo: asyncapi/website PR: 3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: akshatnema
Repo: asyncapi/website PR: 3378
File: scripts/markdown/check-markdown.js:1-1
Timestamp: 2024-11-25T18:34:51.303Z
Learning: When reviewing `scripts/markdown/check-markdown.js`, optimizations should be addressed in separate issues and not included in the current pull request.
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/newsroom/FeaturedBlogPost.tsx:90-92
Timestamp: 2024-10-11T07:38:35.745Z
Learning: In Next.js, nested `<a>` tags cause hydration issues due to invalid HTML. To fix this, avoid nesting `<a>` tags by replacing inner `<a>` tags with non-interactive elements like `<span>`, and use click handlers to maintain interactivity if needed.
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/Avatar.tsx:35-44
Timestamp: 2024-10-11T10:46:58.882Z
Learning: In Next.js, when avoiding nested `<a>` tags due to hydration issues, use accessible non-link elements like `<button>` or `<span>` with appropriate roles, `tabIndex`, and event handlers to maintain accessibility and SEO.
📚 Learning: 2025-01-18T08:44:43.614Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is enforced by the project's configuration which uses `moduleResolution: "bundler"` in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
Applied to files:
next-env.d.ts
📚 Learning: 2025-04-20T16:05:16.482Z
Learnt from: anshgoyalevil
Repo: asyncapi/website PR: 3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Applied to files:
next-env.d.tspages/_app.tsxpages/_document.tsx
📚 Learning: 2025-01-18T08:44:43.614Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is a project-specific requirement enforced by the linting rules and build setup, even though it differs from typical Node.js behavior.
Applied to files:
next-env.d.ts
📚 Learning: 2024-10-11T07:38:35.745Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/newsroom/FeaturedBlogPost.tsx:90-92
Timestamp: 2024-10-11T07:38:35.745Z
Learning: In Next.js, nested `<a>` tags cause hydration issues due to invalid HTML. To fix this, avoid nesting `<a>` tags by replacing inner `<a>` tags with non-interactive elements like `<span>`, and use click handlers to maintain interactivity if needed.
Applied to files:
pages/_app.tsxpages/_document.tsx
📚 Learning: 2024-10-11T10:46:58.882Z
Learnt from: akshatnema
Repo: asyncapi/website PR: 3262
File: components/Avatar.tsx:35-44
Timestamp: 2024-10-11T10:46:58.882Z
Learning: In Next.js, when avoiding nested `<a>` tags due to hydration issues, use accessible non-link elements like `<button>` or `<span>` with appropriate roles, `tabIndex`, and event handlers to maintain accessibility and SEO.
Applied to files:
pages/_app.tsxpages/_document.tsx
📚 Learning: 2024-11-11T21:30:32.478Z
Learnt from: amanbhoria
Repo: asyncapi/website PR: 3373
File: components/AlgoliaSearch.tsx:313-313
Timestamp: 2024-11-11T21:30:32.478Z
Learning: In the `SearchButton` component within `components/AlgoliaSearch.tsx`, if the component re-renders on every button click and the `useEffect` runs accordingly, adding dependencies to the dependency array might not be necessary.
Applied to files:
pages/_app.tsx
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lighthouse CI
🔇 Additional comments (4)
pages/_document.tsx (2)
5-5: Import looks good.The Script import from
next/scriptis correctly added to support the Script component usage below.
56-59: Remove duplicate Script component and move out of Head.The Script component is incorrectly placed inside
<Head>(lines 56-59). Next.js Script components must be placed outside<Head>to function correctly and will otherwise be ignored or cause errors. Additionally,https://buttons.github.io/buttons.jsis already loaded inpages/_app.tsxat line 27, creating duplicate loading. Remove this Script from_document.tsxor consolidate to a single location outside the<Head>component.⛔ Skipped due to learnings
Learnt from: akshatnema Repo: asyncapi/website PR: 3378 File: scripts/markdown/check-markdown.js:1-1 Timestamp: 2024-11-25T18:34:51.303Z Learning: When reviewing `scripts/markdown/check-markdown.js`, optimizations should be addressed in separate issues and not included in the current pull request.Learnt from: anshgoyalevil Repo: asyncapi/website PR: 3950 File: scripts/utils/check-locales.ts:122-129 Timestamp: 2025-04-20T16:05:16.482Z Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.Learnt from: anshgoyalevil Repo: asyncapi/website PR: 3301 File: scripts/markdown/check-markdown.js:0-0 Timestamp: 2024-10-18T17:24:45.053Z Learning: In `scripts/markdown/check-markdown.js`, the script is not run in an asynchronous environment, so refactoring it to use `async/await` is unnecessary.Learnt from: akshatnema Repo: asyncapi/website PR: 3262 File: components/newsroom/FeaturedBlogPost.tsx:90-92 Timestamp: 2024-10-11T07:38:35.745Z Learning: In Next.js, nested `<a>` tags cause hydration issues due to invalid HTML. To fix this, avoid nesting `<a>` tags by replacing inner `<a>` tags with non-interactive elements like `<span>`, and use click handlers to maintain interactivity if needed.Learnt from: akshatnema Repo: asyncapi/website PR: 3262 File: components/Avatar.tsx:35-44 Timestamp: 2024-10-11T10:46:58.882Z Learning: In Next.js, when avoiding nested `<a>` tags due to hydration issues, use accessible non-link elements like `<button>` or `<span>` with appropriate roles, `tabIndex`, and event handlers to maintain accessibility and SEO.next-env.d.ts (1)
1-6: Standard Next.js TypeScript configuration file.This is the standard auto-generated Next.js environment declaration file with appropriate type references for Next.js core types, image types, and route types. The file structure and content are correct.
pages/_app.tsx (1)
16-16: Import looks good.The Script import from
next/scriptis correctly added.
Comment on lines
+25
to
+28
| <Script | ||
| strategy="afterInteractive" | ||
| src="https://buttons.github.io/buttons.js" | ||
| /> |
There was a problem hiding this comment.
Duplicate script loading and incorrect placement.
Same issues as in pages/_document.tsx:
-
Script in Head: The Script component should not be inside
<Head>. Place it in the component body instead. -
Duplicate loading: This script is already loaded in
pages/_document.tsxat lines 56-59. Loading it in both files is redundant and inefficient.
Recommendation: Since this is an application-level script for GitHub buttons, consider:
- Keeping it only in
_app.tsx(remove from_document.tsx) - Moving it outside the
<Head>component to the component body (e.g., place it before or after the<AlgoliaSearch>wrapper)
Apply this diff to fix the placement:
{/* <MDXProvider components={mdxComponents}> */}
<Head>
- <Script
- strategy="afterInteractive"
- src="https://buttons.github.io/buttons.js"
- />
</Head>
+ <Script
+ strategy="afterInteractive"
+ src="https://buttons.github.io/buttons.js"
+ />
<AlgoliaSearch>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
pages/_app.tsx around lines 25-28: The GitHub buttons Script is incorrectly
placed inside Head and duplicates the same script loaded in pages/_document.tsx;
remove this Script from _app.tsx (or conversely remove it from _document.tsx) so
it only exists once in the app, and if you keep it in _app.tsx move the Script
component out of Head into the component body (e.g., just before or after the
<AlgoliaSearch> wrapper) using strategy="afterInteractive" and the same src.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Priority: LOW
Description
scripttag withScriptfromnext/scriptstrategy="afterInteractive"should behave asasync deferattirbutenext/script docs: https://nextjs.org/docs/pages/api-reference/components/script
This resolves the warning of
Do not add <script> tags using next/head.Resolves #4517
Summary by CodeRabbit