[DevOverlay] Align old and new overlay by devjiwonchoi · Pull Request #74935 · vercel/next.js

devjiwonchoi · 2025-01-15T16:22:00Z

Enable the new UI for the CI testings of existing redbox tests.

There are several changes made to let the test pass, including backporting changes to the old UI or removing one from the new, and are as follows:

Added back | after line number in code frame (link)
Removed unnecessary @ in Terminal component (link)
Set open overlay default value to true in Pages Router (link)
Backport displaying the first first-party call stack frame to the CallStack component (link)
Move the devTools component back to the error boundary (link)
- Was moved out at [DevOverlay] Decouple Error Overlay with DevTools Indicator #74999 (comment)

Closes NDX-674
Closes NDX-687

devjiwonchoi · 2025-01-15T16:22:22Z

[DevOverlay] Align old and new overlay #74935 👈 (View in Graphite)
[DevOverlay] Enable reactOwnerStack when newDevOverlay is enabled #75199
canary

This stack of pull requests is managed by Graphite. Learn more about stacking.

ijjk · 2025-01-15T16:25:58Z

Failing test suites

Commit: 2334de2

pnpm test-dev-turbo test/e2e/app-dir/app-compilation/index.test.ts (turbopack)

app dir > HMR > should not cause error when removing loading.js

Expand output

● app dir › HMR › should not cause error when removing loading.js

expect(received).toInclude(expected)

Expected string to include:
  "✓ Compiled"
Received:
  " ○ Compiling /page-with-loading ...
"

  38 |
  39 |         await retry(async () => {
> 40 |           expect(next.cliOutput.slice(cliOutputLength)).toInclude('✓ Compiled')
     |                                                         ^
  41 |         })
  42 |
  43 |         // It should not have an error

  at toInclude (e2e/app-dir/app-compilation/index.test.ts:40:57)
  at fn (lib/next-test-utils.ts:810:20)
  at Object.<anonymous> (e2e/app-dir/app-compilation/index.test.ts:39:9)

Read more about building and testing Next.js in contributing.md.

ijjk · 2025-01-15T16:29:58Z

Stats from current PR

Default Build

General Overall increase ⚠️

	vercel/next.js canary	vercel/next.js 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled	Change
buildDuration	19.2s	17.6s	N/A
buildDurationCached	16.8s	14.1s	N/A
nodeModulesSize	419 MB	419 MB	⚠️ +8.8 kB
nextStartRea..uration (ms)	447ms	446ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	vercel/next.js 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled	Change
5306-HASH.js gzip	54.1 kB	54.1 kB	N/A
8276.HASH.js gzip	169 B	168 B	N/A
8377-HASH.js gzip	5.46 kB	5.46 kB	N/A
bccd1874-HASH.js gzip	52.9 kB	52.9 kB	N/A
framework-HASH.js gzip	57.5 kB	57.5 kB	N/A
main-app-HASH.js gzip	241 B	242 B	N/A
main-HASH.js gzip	34.6 kB	34.6 kB	N/A
webpack-HASH.js gzip	1.71 kB	1.71 kB	N/A
Overall change	0 B	0 B	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	vercel/next.js 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled	Change
polyfills-HASH.js gzip	39.4 kB	39.4 kB	✓
Overall change	39.4 kB	39.4 kB	✓

Client Pages

	vercel/next.js canary	vercel/next.js 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled	Change
_app-HASH.js gzip	193 B	193 B	✓
_error-HASH.js gzip	193 B	193 B	✓
amp-HASH.js gzip	512 B	510 B	N/A
css-HASH.js gzip	343 B	342 B	N/A
dynamic-HASH.js gzip	1.84 kB	1.84 kB	✓
edge-ssr-HASH.js gzip	265 B	265 B	✓
head-HASH.js gzip	363 B	362 B	N/A
hooks-HASH.js gzip	393 B	392 B	N/A
image-HASH.js gzip	4.59 kB	4.58 kB	N/A
index-HASH.js gzip	268 B	268 B	✓
link-HASH.js gzip	2.35 kB	2.35 kB	N/A
routerDirect..HASH.js gzip	328 B	328 B	✓
script-HASH.js gzip	397 B	397 B	✓
withRouter-HASH.js gzip	323 B	326 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	3.59 kB	3.59 kB	✓

Client Build Manifests

	vercel/next.js canary	vercel/next.js 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled	Change
_buildManifest.js gzip	748 B	747 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled	Change
index.html gzip	523 B	524 B	N/A
link.html gzip	539 B	537 B	N/A
withRouter.html gzip	520 B	520 B	✓
Overall change	520 B	520 B	✓

Edge SSR bundle Size

	vercel/next.js canary	vercel/next.js 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled	Change
edge-ssr.js gzip	129 kB	129 kB	N/A
page.js gzip	210 kB	210 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	vercel/next.js 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled	Change
middleware-b..fest.js gzip	670 B	666 B	N/A
middleware-r..fest.js gzip	155 B	156 B	N/A
middleware.js gzip	31.3 kB	31.3 kB	N/A
edge-runtime..pack.js gzip	844 B	844 B	✓
Overall change	844 B	844 B	✓

Next Runtimes

	vercel/next.js canary	vercel/next.js 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled	Change
274-experime...dev.js gzip	322 B	322 B	✓
274.runtime.dev.js gzip	314 B	314 B	✓
app-page-exp...dev.js gzip	378 kB	378 kB	N/A
app-page-exp..prod.js gzip	132 kB	132 kB	✓
app-page-tur..prod.js gzip	145 kB	145 kB	✓
app-page-tur..prod.js gzip	141 kB	141 kB	✓
app-page.run...dev.js gzip	365 kB	365 kB	N/A
app-page.run..prod.js gzip	128 kB	128 kB	✓
app-route-ex...dev.js gzip	37.6 kB	37.6 kB	✓
app-route-ex..prod.js gzip	25.6 kB	25.6 kB	✓
app-route-tu..prod.js gzip	25.6 kB	25.6 kB	✓
app-route-tu..prod.js gzip	25.4 kB	25.4 kB	✓
app-route.ru...dev.js gzip	39.2 kB	39.2 kB	✓
app-route.ru..prod.js gzip	25.4 kB	25.4 kB	✓
pages-api-tu..prod.js gzip	9.69 kB	9.69 kB	✓
pages-api.ru...dev.js gzip	11.6 kB	11.6 kB	✓
pages-api.ru..prod.js gzip	9.68 kB	9.68 kB	✓
pages-turbo...prod.js gzip	21.9 kB	21.9 kB	✓
pages.runtim...dev.js gzip	27.7 kB	27.7 kB	✓
pages.runtim..prod.js gzip	21.9 kB	21.9 kB	✓
server.runti..prod.js gzip	916 kB	916 kB	✓
Overall change	1.74 MB	1.74 MB	✓

build cache

	vercel/next.js canary	vercel/next.js 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled	Change
0.pack gzip	2.1 MB	2.1 MB	N/A
index.pack gzip	74.9 kB	74.9 kB	N/A
Overall change	0 B	0 B	✓

Diff details

Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js

failed to diff

Diff for app-page.runtime.dev.js

failed to diff

Commit: 2334de2

devjiwonchoi · 2025-01-22T17:44:03Z

-              line
-                .substring(a + 1)
-                .replace(`^\\ {${miniLeadingSpacesLength}}`, '')
+              line.substring(a).replace(`^\\ {${miniLeadingSpacesLength}}`, '')


Don't remove | from the code frame after the line number, as it increases too many conditional snapshot testings. This is the original behavior from the old UI.

next.js/packages/next/src/client/components/react-dev-overlay/internal/components/CodeFrame/CodeFrame.tsx

Line 37 in ab87453

line.substring(a).replace(`^\\ {${miniLeadingSpacesLength}}`, '')

devjiwonchoi · 2025-01-22T17:45:03Z

-            {getFrameSource(stackFrame)} @{' '}
-            <HotlinkedText text={stackFrame.methodName} />
+            {getFrameSource(stackFrame)}
+            {/* TODO: Unlike the CodeFrame component, the `methodName` is unavailable. */}


Remove unnecessary @ from the Terminal component, as we won't know the method name from the build error. This is the behavior from the old UI.

next.js/packages/next/src/client/components/react-dev-overlay/internal/components/Terminal/Terminal.tsx

Lines 75 to 82 in ab87453

{file && (

<EditorLink

isSourceFile

key={file.fileName}

file={file.fileName}

location={file.location}

/>

)}

devjiwonchoi · 2025-01-22T17:45:47Z

    isAppDir: false,
  })

+  const [isErrorOverlayOpen, setIsErrorOverlayOpen] = useState(true)


Set default value of isErrorOverlayOpen to true. This is the behavior from the old UI.

next.js/packages/next/src/client/components/react-dev-overlay/pages/old-react-dev-overlay.tsx

Line 48 in ab87453

initialDisplayState={'fullscreen'}

devjiwonchoi · 2025-01-22T17:48:20Z

          : filteredFrames.slice(0, firstFirstPartyFrameIndex),
      trailingCallStackFrames: filteredFrames.slice(
-        firstFirstPartyFrameIndex + 1
+        firstFirstPartyFrameIndex < 0 ? 0 : firstFirstPartyFrameIndex


This is the old UI, backporting the behavior to include the first first-party call stack frame to be included in the CallStack component. Previously, it was only displayed at the CodeFrame.

next.js/packages/next/src/client/components/react-dev-overlay/_experimental/internal/container/runtime-error/index.tsx

Lines 12 to 23 in ab87453

const { firstFrame } = useMemo(() => {

const firstFirstPartyFrameIndex = error.frames.findIndex(

(entry) =>

!entry.ignored &&

Boolean(entry.originalCodeFrame) &&

Boolean(entry.originalStackFrame)

)

return {

firstFrame: error.frames[firstFirstPartyFrameIndex] ?? null,

}

}, [error.frames])

devjiwonchoi · 2025-01-22T17:49:19Z

-    </>
+  return (
+    <DevToolsErrorBoundary
+      devTools={devTools}


Should pass the devTools to the error boundary when global error.

huozhi · 2025-01-22T20:26:38Z

-      >
-        {children}
-      </DevToolsErrorBoundary>
+  const devTools = (


we should call it error overlay as it's only containing overlay related thing.

eps1lon

Adjusted PR title. Previous one made it sound like it's testing only

huozhi · 2025-01-23T18:16:36Z

why this is different on PPR? can we just align the data attributes?

On old, on click toast opened the overlay, now need to click the correct element that has "open issue" action, since there's "dismiss" button included. __NEXT_EXPERIMENTAL_PPR is a mistake.

huozhi · 2025-01-23T18:19:23Z

why we need another flag __NEXT_EXPERIMENTAL_NEW_DEV_OVERLAY set in CI, shouldn't it just be __NEXT_EXPERIMENTAL_PPR === true? the variable on nextjs side is all set by define plugin

x-ref: #74935 (comment)

__NEXT_EXPERIMENTAL_PPR is feasible as it's only for testing, but I think explicit __NEXT_EXPERIMENTAL_NEW_DEV_OVERLAY would be better to isolate the condition in case needed.

ijjk added created-by: Next.js team PRs by the Next.js team. type: next labels Jan 15, 2025

This was referenced Jan 15, 2025

[DevOverlay] Fix floating header invisble on small screen #74886

Merged

[DevOverlay] Keep pagination on Build Error #74905

Merged

devjiwonchoi mentioned this pull request Jan 15, 2025

[DevOverlay] Gather Feedback per Error #74908

Merged

ijjk added the tests label Jan 16, 2025

devjiwonchoi force-pushed the 01-15-_devoverlay_gather_feedback_per_error branch 3 times, most recently from 279cbd2 to 003a61d Compare January 16, 2025 16:43

devjiwonchoi changed the base branch from 01-15-_devoverlay_gather_feedback_per_error to graphite-base/74935 January 16, 2025 17:00

devjiwonchoi force-pushed the 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled branch from 7d9c834 to f477735 Compare January 16, 2025 17:13

devjiwonchoi force-pushed the graphite-base/74935 branch from 003a61d to 4a0c516 Compare January 16, 2025 17:13

devjiwonchoi changed the base branch from graphite-base/74935 to canary January 16, 2025 17:13

devjiwonchoi force-pushed the 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled branch from f477735 to ee2754d Compare January 16, 2025 17:18

devjiwonchoi mentioned this pull request Jan 16, 2025

[DevOverlay] Decouple Error Overlay with DevTools Indicator #74999

Merged

devjiwonchoi force-pushed the 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled branch from ee2754d to 7c8e142 Compare January 16, 2025 19:32

devjiwonchoi changed the base branch from canary to 01-17-_devoverlay_decouple_error_overlay_with_devtools_indicator January 16, 2025 19:32

devjiwonchoi mentioned this pull request Jan 16, 2025

[DevOverlay] Rename file names to kebab #75000

Merged

devjiwonchoi force-pushed the 01-17-_devoverlay_decouple_error_overlay_with_devtools_indicator branch from c671e8a to ddf157e Compare January 16, 2025 21:14

devjiwonchoi force-pushed the 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled branch from 7c8e142 to 7b6ba6e Compare January 16, 2025 21:14

devjiwonchoi force-pushed the 01-17-_devoverlay_decouple_error_overlay_with_devtools_indicator branch 2 times, most recently from 970daea to 8bea48c Compare January 16, 2025 21:40

devjiwonchoi force-pushed the 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled branch from 7b6ba6e to e6d41c4 Compare January 16, 2025 21:40

devjiwonchoi force-pushed the 01-17-_devoverlay_decouple_error_overlay_with_devtools_indicator branch from 8bea48c to ac0f76f Compare January 17, 2025 00:38

devjiwonchoi force-pushed the 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled branch 2 times, most recently from 6649e86 to f2f1850 Compare January 17, 2025 00:52

devjiwonchoi force-pushed the 01-17-_devoverlay_decouple_error_overlay_with_devtools_indicator branch from 0d43d19 to 05b4b00 Compare January 17, 2025 08:53

devjiwonchoi force-pushed the 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled branch from b435757 to e3528dc Compare January 21, 2025 05:02

devjiwonchoi changed the base branch from graphite-base/74935 to canary January 21, 2025 05:02

devjiwonchoi force-pushed the 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled branch 2 times, most recently from b638ddd to c420ed7 Compare January 22, 2025 17:20

devjiwonchoi commented Jan 22, 2025

View reviewed changes

devjiwonchoi marked this pull request as ready for review January 22, 2025 17:54

devjiwonchoi requested review from eps1lon, gaojude, huozhi and ztanner January 22, 2025 18:03

gaojude approved these changes Jan 22, 2025

View reviewed changes

devjiwonchoi mentioned this pull request Jan 22, 2025

[DevOverlay] Enable reactOwnerStack when newDevOverlay is enabled #75199

Merged

huozhi reviewed Jan 22, 2025

View reviewed changes

devjiwonchoi force-pushed the 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled branch 2 times, most recently from a46dd39 to 581c376 Compare January 23, 2025 06:36

eps1lon changed the title ~~[DevOverlay] Enable new UI when PPR testing is enabled~~ [DevOverlay] Align old and new overlay Jan 23, 2025

eps1lon reviewed Jan 23, 2025

View reviewed changes

Comment thread packages/next/src/build/webpack/plugins/define-env-plugin.ts Outdated

devjiwonchoi force-pushed the 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled branch from 3c09148 to e12030b Compare January 23, 2025 14:39

devjiwonchoi changed the base branch from canary to 01-23-_devoverlay_enable_reactownerstack_when_newdevoverlay_is_enabled January 23, 2025 14:39

devjiwonchoi force-pushed the 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled branch 2 times, most recently from 288c3d0 to eff3089 Compare January 23, 2025 15:27

huozhi reviewed Jan 23, 2025

View reviewed changes

huozhi approved these changes Jan 23, 2025

View reviewed changes

Base automatically changed from 01-23-_devoverlay_enable_reactownerstack_when_newdevoverlay_is_enabled to canary January 23, 2025 21:07

[DevOverlay] Enable new UI when PPR testing is enabled

2334de2

devjiwonchoi force-pushed the 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled branch from 655b867 to 2334de2 Compare January 23, 2025 21:17

devjiwonchoi merged commit baa4f78 into canary Jan 23, 2025

devjiwonchoi deleted the 01-16-_devoverlay_enable_new_ui_when_ppr_testing_is_enabled branch January 23, 2025 21:53

github-actions Bot added the locked label Feb 7, 2025

github-actions Bot locked as resolved and limited conversation to collaborators Feb 7, 2025

	{file && (
	<EditorLink
	isSourceFile
	key={file.fileName}
	file={file.fileName}
	location={file.location}
	/>
	)}

	const { firstFrame } = useMemo(() => {
	const firstFirstPartyFrameIndex = error.frames.findIndex(
	(entry) =>
	!entry.ignored &&
	Boolean(entry.originalCodeFrame) &&
	Boolean(entry.originalStackFrame)
	)

	return {
	firstFrame: error.frames[firstFirstPartyFrameIndex] ?? null,
	}
	}, [error.frames])

Conversation

devjiwonchoi commented Jan 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devjiwonchoi commented Jan 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ijjk commented Jan 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Failing test suites

Uh oh!

ijjk commented Jan 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Stats from current PR

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devjiwonchoi Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eps1lon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

devjiwonchoi commented Jan 15, 2025 •

edited

Loading

devjiwonchoi commented Jan 15, 2025 •

edited

Loading

ijjk commented Jan 15, 2025 •

edited

Loading

ijjk commented Jan 15, 2025 •

edited

Loading

devjiwonchoi Jan 22, 2025 •

edited

Loading