-
Notifications
You must be signed in to change notification settings - Fork 12k
fix(ui): migrate off Next.js <Link legacyBehavior> — fix hydration & ref-forwarding issues #24418
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.
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
Changes from all commits
57ed394
6a4d93e
df4955c
71bad74
b237a12
807c51b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| # Bug report: Migrate off Next.js <Link legacyBehavior> — hydration, ref-forwarding, and runtime fixes | ||
|
|
||
| ### Issue Summary | ||
| One-sentence explanation: Next.js `Link` components using `legacyBehavior` caused DOM nesting, ref-forwarding and hydration/runtime errors (e.g. `null parentNode`); this PR migrates `Link` usage to the new API and fixes the related issues. | ||
|
|
||
| --- | ||
|
|
||
| ### Steps to Reproduce | ||
| 1. Prepare the repository and dependencies: | ||
| - `yarn install && yarn dedupe` | ||
| 2. Start the app in dev or production build: | ||
| - `yarn dev` (or `yarn build && yarn start`) | ||
| 3. Open pages that use Link / interactive components such as: | ||
| - Bookings single view, Forgot-password single view, AppNotInstalledMessage, Make & Zapier setup pages, pages with Dropdowns/Buttons-as-Links/List/Stepper | ||
| 4. Interact and observe: | ||
| - Watch the browser console for hydration warnings and runtime errors when opening dropdowns or navigating via keyboard. | ||
| 5. Reproduce specific runtime error path: | ||
| - Trigger flows that convert HTML → markdown (turndownService) or render server/client inconsistent Link markup to reproduce `Cannot read properties of null (reading 'parentNode')` or hydration mismatch. | ||
|
|
||
| Any other relevant information: this is a bug because `legacyBehavior` produced inconsistent server vs client HTML structure and unsafe ref usage, causing developer-facing console warnings and potential user-facing UI breakage. Expected behavior is deterministic SSR → client render with correct element types, correct ref forwarding, and preserved accessibility (keyboard focus, ARIA). | ||
|
|
||
| --- | ||
|
|
||
| ### Actual Results | ||
| - Hydration mismatch warnings appear in the browser console on initial page load. | ||
| - Runtime errors such as `Cannot read properties of null (reading 'parentNode')` from `turndownService` in certain flows. | ||
| - Broken ref forwarding and nested interactive elements (anchor inside button or vice-versa) leading to unexpected keyboard behavior and accessibility regressions. | ||
| - ESLint warnings related to unsafe `any` casts and unused imports in affected files. | ||
|
|
||
| --- | ||
|
|
||
| ### Expected Results | ||
| - No hydration mismatch warnings on page load. | ||
| - No runtime exceptions from `turndownService` or component render paths. | ||
| - Correct ref forwarding (use of `forwardRef` where needed) and no nested interactive elements. | ||
| - Keyboard interaction and focus behavior remain correct and accessible. | ||
| - Clean lint/type output for the modified files. | ||
|
|
||
| --- | ||
|
|
||
| ### Technical details | ||
| - Environment used for testing: macOS (local dev), Node.js 18+ recommended, Yarn | ||
| - Commands used: | ||
| - `yarn install && yarn dedupe` | ||
| - `yarn dev` | ||
| - `yarn build && yarn start` | ||
| - `yarn lint` | ||
| - `yarn test` | ||
| - Branch with fixes: `fix/next-link-hydration` | ||
| - Key files changed: | ||
| - `packages/ui/components/dropdown/Dropdown.tsx` | ||
| - `packages/ui/components/button/Button.tsx` | ||
| - `packages/ui/components/list/List.tsx` | ||
| - `packages/ui/components/form/step/Stepper.tsx` | ||
| - `packages/lib/turndownService.ts` | ||
| - `packages/features/shell/CalAiBanner.tsx` | ||
| - `packages/app-store/_components/AppNotInstalledMessage.tsx` | ||
| - `apps/web/modules/bookings/views/bookings-single-view.tsx` | ||
| - `apps/web/components/apps/make/Setup.tsx` | ||
| - `apps/web/components/apps/zapier/Setup.tsx` | ||
| - `apps/web/modules/auth/forgot-password/[id]/forgot-password-single-view.tsx` | ||
|
|
||
| --- | ||
|
|
||
| ### Evidence | ||
| - Manual tests performed: | ||
| - Dev and production builds: confirmed hydration warnings present before the change and resolved after migration to the new `Link` API. | ||
| - Interacted with Dropdowns, List items, Buttons-as-Links, and Stepper to validate keyboard navigation and no nested anchors/buttons. | ||
| - Reproduced and fixed the `null parentNode` error path in `turndownService`. | ||
| - Attachments to include with the issue/PR: | ||
| - Console screenshot showing hydration warning(s) (before) | ||
| - Short GIF showing dropdown/button behavior before vs after (recommended) | ||
| - Relevant console stack trace(s) for the runtime exception(s) | ||
| - Testing notes: run the smoke test checklist in `PR_DESCRIPTION.md` and verify CI (typecheck/lint/tests) passes. | ||
|
|
||
| --- | ||
|
|
||
| (You can copy-paste the content above into GitHub's "New Issue" form and attach the screenshots/GIFs for stronger evidence.) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| Summary | ||
| I encountered several runtime and hydration issues when running the app locally that traced back to usage of Next.js <Link legacyBehavior>. This repo-wide migration removes legacyBehavior usage and fixes ref-forwarding, DOM nesting (nested anchors/buttons), hydration mismatch warnings, and a runtime null parentNode error in turndownService. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrap Prompt for AI agents |
||
|
|
||
| Why this matters | ||
| - legacyBehavior produces incorrect DOM nesting and broken refs on modern React/Next versions, causing hydration warnings and runtime errors. | ||
| - Migrating ensures compatibility with React 19 / Next.js 15 and prevents user-facing regressions. | ||
|
|
||
| What the PR changes | ||
| - Removes all uses of Link legacyBehavior and updates Link usage to the new API. | ||
| - Fixes ref forwarding and removes unsafe any casts in Dropdown and Button. | ||
| - Splits List to make SSR deterministic and introduced ListLinkItem to avoid nested interactive elements. | ||
| - Hardened turndownService to avoid traversing null parent nodes. | ||
| - Deferred CalAiBanner visibility to client mount to prevent hydration mismatch. | ||
| - Cleaned up ESLint warnings and minor typing fixes. | ||
|
|
||
| How to reproduce (developer-local) | ||
| 1) yarn install && yarn dedupe | ||
| 2) yarn dev (or build with yarn build) | ||
| 3) Observe console/hydration warnings and runtime errors during flows using components listed above (Dropdowns, Buttons-as-Links, List items, Stepper). | ||
| 4) Apply the changes on branch fix/next-link-hydration and verify warnings/errors are gone. | ||
|
|
||
| Files changed (high-level) | ||
| - packages/ui/components/dropdown/Dropdown.tsx | ||
| - packages/ui/components/button/Button.tsx | ||
| - packages/ui/components/list/List.tsx | ||
| - packages/ui/components/form/step/Stepper.tsx | ||
| - packages/lib/turndownService.ts | ||
| - packages/features/shell/CalAiBanner.tsx | ||
| - packages/app-store/_components/AppNotInstalledMessage.tsx | ||
| - apps/web/modules/bookings/views/bookings-single-view.tsx | ||
| - apps/web/components/apps/make/Setup.tsx | ||
| - apps/web/components/apps/zapier/Setup.tsx | ||
| - apps/web/modules/auth/forgot-password/[id]/forgot-password-single-view.tsx | ||
|
|
||
| Notes | ||
| - No DB/API changes. This was discovered while running the app locally; no prior issue existed. Please review for regressions in keyboard/focus/ARIA behavior. | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,91 @@ | ||||||
| fix(ui): migrate off Next.js <Link legacyBehavior> — fix hydration & ref-forwarding issues | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Escape Prompt for AI agents
Suggested change
|
||||||
|
|
||||||
| Fixes | ||||||
| Fixes #24417 | ||||||
|
|
||||||
| Summary | ||||||
| - Removed deprecated Next.js `Link` `legacyBehavior` usage across the codebase and migrated affected components to the new `Link` API to resolve ref-forwarding, DOM nesting, and hydration/runtime errors observed while running the application locally. | ||||||
| - Fixed runtime errors (e.g. `null parentNode`), improved SSR determinism, and preserved accessibility/interactive behavior for core UI components (Dropdown, Button, List, Stepper, etc.). | ||||||
|
|
||||||
| Impact / Scope | ||||||
| - UI-only, non-breaking changes. No API or DB changes. Small typing and lint fixes included. | ||||||
|
|
||||||
| Motivation | ||||||
| Using `legacyBehavior` produced inconsistent server/client markup, broken refs, and nested interactive elements (anchor inside button), leading to hydration warnings and runtime exceptions. Migrating to the new `Link` API prevents these problems and prepares the codebase for React 19 / Next.js 15. | ||||||
|
|
||||||
| Files changed (high level) | ||||||
| - `packages/ui/components/dropdown/Dropdown.tsx` | ||||||
| - `packages/ui/components/button/Button.tsx` | ||||||
| - `packages/ui/components/list/List.tsx` | ||||||
| - `packages/ui/components/form/step/Stepper.tsx` | ||||||
| - `packages/lib/turndownService.ts` | ||||||
| - `packages/features/shell/CalAiBanner.tsx` | ||||||
| - `packages/app-store/_components/AppNotInstalledMessage.tsx` | ||||||
| - `apps/web/modules/bookings/views/bookings-single-view.tsx` | ||||||
| - `apps/web/components/apps/make/Setup.tsx` | ||||||
| - `apps/web/components/apps/zapier/Setup.tsx` | ||||||
| - `apps/web/modules/auth/forgot-password/[id]/forgot-password-single-view.tsx` | ||||||
| - plus small lint/type fixes across related files | ||||||
|
|
||||||
| Key changes | ||||||
| - Removed all uses of Link `legacyBehavior` and updated Link usage to the new API (pass `className`/props directly to `Link`). | ||||||
| - Fixed ref forwarding and removed unsafe `any` casts in Dropdown and Button components. | ||||||
| - Separated List into SSR-safe rendering and a `ListLinkItem` to avoid nested interactive elements. | ||||||
| - Hardened `turndownService` to avoid traversing null parent nodes. | ||||||
| - Deferred `CalAiBanner` visibility to client mount to prevent hydration mismatches. | ||||||
| - Cleaned up ESLint warnings and added targeted rule disables where necessary. | ||||||
|
|
||||||
| Manual QA steps (recommended) | ||||||
| 1) Prepare | ||||||
| - `yarn install && yarn dedupe` | ||||||
| 2) Local checks | ||||||
| - `yarn dev` # or build with `yarn build` | ||||||
| - `yarn lint` | ||||||
| - `yarn test` | ||||||
| 3) Smoke tests (verify in both dev and production builds) | ||||||
| - Dropdown menus: open/close, keyboard navigation, focus behavior | ||||||
| - Button components rendering as Links: ensure no nested anchors/buttons and keyboard/Enter works | ||||||
| - List + ListLinkItem: SSR output matches client render and navigation works | ||||||
| - Stepper: next/back flow | ||||||
| - Bookings single view, Forgot-password view, AppNotInstalledMessage, Make & Zapier setup pages: links/navigation work | ||||||
| - CalAiBanner: no hydration mismatch on initial page load | ||||||
| 4) Edge cases | ||||||
| - Keyboard-only navigation and screen-reader checks for major components | ||||||
| - Confirm prior runtime errors (`null parentNode`, ref issues) no longer appear in logs/console | ||||||
|
|
||||||
| What reviewers should verify | ||||||
| - PR title follows Conventional Commits (starts with `fix:`) — required for CI | ||||||
| - No remaining usage of `legacyBehavior` | ||||||
| - Correct ref forwarding and typing (no unsafe `any` casts) | ||||||
| - No nested interactive elements introduced by the changes | ||||||
| - No hydration mismatch warnings in browser console | ||||||
| - CI (typecheck / lint / tests) passes | ||||||
| - Behavioral parity with previous UI (keyboard/focus/ARIA preserved) | ||||||
|
|
||||||
| Commands (copy to terminal) | ||||||
| # Update PR title (run once) | ||||||
| gh pr edit 24417 --title "fix(ui): migrate off Next.js <Link legacyBehavior> — fix hydration & ref-forwarding issues" | ||||||
|
|
||||||
| # Update PR body from this file (run once) | ||||||
| gh pr edit 24417 --body-file PR_DESCRIPTION.md | ||||||
|
|
||||||
| # Optional: add reviewers/labels | ||||||
| gh pr edit 24417 --add-reviewer maintainer1 --add-reviewer maintainer2 --add-label "area:ui" --add-label "type:bug" | ||||||
|
|
||||||
| # Local checks | ||||||
| yarn install && yarn dedupe | ||||||
| yarn build | ||||||
| yarn lint | ||||||
| yarn test | ||||||
|
|
||||||
| Visual demo (strongly recommended) | ||||||
| - Attach a console screenshot showing the hydration warning (before) and a short GIF (5–10s) showing dropdown/button behavior before vs after. Upload via the PR web UI. | ||||||
|
|
||||||
| Checklist (DO NOT REMOVE) | ||||||
| - [ ] I have self-reviewed the code | ||||||
| - [ ] I have added tests where applicable, or noted why not applicable | ||||||
| - [ ] I have run lint and fixed warnings where possible | ||||||
| - [ ] I have added a visual demo (recommended) or documented how to reproduce | ||||||
|
|
||||||
| Notes | ||||||
| - No API/DB changes; non-breaking UI fixes. This PR fixes issues reported in #24417 and was created after encountering the issues while running the application locally. | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix heading hierarchy to satisfy markdownlint
After the top-level
#heading, the next heading jumps straight to###, triggering MD001. Please promote the section headings to H2 so the hierarchy increments by one level.📝 Committable suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents