Skip to content

Commit

Permalink
Fix indentation in source code of dev overlay (#61216)
Browse files Browse the repository at this point in the history
This fix the bad missing indentation for the source code in dev overlay.
It was caused by a space replacement change in dev overlay before. Which
should only replace the leading multi-spaces but it was replacing other
spaces as well in the code.

#### Code

```jsx
export default async function Page() {
  await fetch('http://localhost:3004')
}
```

#### After
<img width="486" alt="image"
src="https://github.com/vercel/next.js/assets/4800338/2c0a2720-c3c8-4db2-a548-f4daabd3c5d5">


#### Before
<img width="484" alt="image"
src="https://github.com/vercel/next.js/assets/4800338/f68ceea0-cead-4e23-9db7-2acb24a14485">



Closes NEXT-2166
Closes NEXT-2257
  • Loading branch information
huozhi committed Jan 27, 2024
1 parent b08bed7 commit 54f3399
Show file tree
Hide file tree
Showing 9 changed files with 157 additions and 151 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ export const CodeFrame: React.FC<CodeFrameProps> = function CodeFrame({
// Strip leading spaces out of the code frame:
const formattedFrame = React.useMemo<string>(() => {
const lines = codeFrame.split(/\r?\n/g)
const prefixLength = lines

// Find the minimum length of leading spaces after `|` in the code frame
const miniLeadingSpacesLength = lines
.map((line) =>
/^>? +\d+ +\| [ ]+/.exec(stripAnsi(line)) === null
? null
Expand All @@ -24,12 +26,14 @@ export const CodeFrame: React.FC<CodeFrameProps> = function CodeFrame({
.map((v) => v!.pop()!)
.reduce((c, n) => (isNaN(c) ? n.length : Math.min(c, n.length)), NaN)

if (prefixLength > 1) {
const p = ' '.repeat(prefixLength)
// When the minimum length of leading spaces is greater than 1, remove them
// from the code frame to help the indentation looks better when there's a lot leading spaces.
if (miniLeadingSpacesLength > 1) {
return lines
.map((line, a) =>
~(a = line.indexOf('|'))
? line.substring(0, a) + line.substring(a).replace(p, '')
? line.substring(0, a) +
line.substring(a).replace(`^\\ {${miniLeadingSpacesLength}}`, '')
: line
)
.join('\n')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ export const CodeFrame: React.FC<CodeFrameProps> = function CodeFrame({
// Strip leading spaces out of the code frame:
const formattedFrame = React.useMemo<string>(() => {
const lines = codeFrame.split(/\r?\n/g)
const prefixLength = lines

// Find the minimum length of leading spaces after `|` in the code frame
const miniLeadingSpacesLength = lines
.map((line) =>
/^>? +\d+ +\| [ ]+/.exec(stripAnsi(line)) === null
? null
Expand All @@ -26,12 +28,16 @@ export const CodeFrame: React.FC<CodeFrameProps> = function CodeFrame({
.map((v) => v!.pop()!)
.reduce((c, n) => (isNaN(c) ? n.length : Math.min(c, n.length)), NaN)

if (prefixLength > 1) {
const p = ' '.repeat(prefixLength)
// When the minimum length of leading spaces is greater than 1, remove them
// from the code frame to help the indentation looks better when there's a lot leading spaces.
if (miniLeadingSpacesLength > 1) {
return lines
.map((line, a) =>
~(a = line.indexOf('|'))
? line.substring(0, a) + line.substring(a).replace(p, '')
? line.substring(0, a) +
line
.substring(a)
.replace(new RegExp(`^\\ {${miniLeadingSpacesLength}}`), '')
: line
)
.join('\n')
Expand Down
13 changes: 6 additions & 7 deletions test/development/acceptance-app/ReactRefreshLogBox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,13 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
export default function Page() {
return (
<>
<p>index page</p>
<p>index page</p>
<a onClick={() => {
throw new Error('idk')
}}>
click me
</a>
<a onClick={() => {
throw new Error('idk')
}}>
click me
</a>
</>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ exports[`ReactRefreshLogBox app default Should not show __webpack_exports__ when
"index.js (3:10) @ default
1 | export default () => {
2 | if (typeof window !== 'undefined') {
> 3 | throw new Error('test')
| ^
4 | }
2 | if (typeof window !== 'undefined') {
> 3 | throw new Error('test')
| ^
4 | }
5 |
6 | return null"
6 | return null"
`;
exports[`ReactRefreshLogBox app default boundaries 1`] = `
Expand All @@ -44,10 +44,10 @@ exports[`ReactRefreshLogBox app default conversion to class component (1) 1`] =
"Child.js (4:10) @ ClickCount.render
2 | export default class ClickCount extends Component {
3 | render() {
> 4 | throw new Error()
| ^
5 | }
3 | render() {
> 4 | throw new Error()
| ^
5 | }
6 | }"
`;
Expand Down Expand Up @@ -81,22 +81,22 @@ exports[`ReactRefreshLogBox app default module init error not shown 1`] = `
1 | // top offset for snapshot
2 | import * as React from 'react';
> 3 | throw new Error('no')
| ^
| ^
4 | class ClassDefault extends React.Component {
5 | render() {
6 | return <h1>Default Export</h1>;"
5 | render() {
6 | return <h1>Default Export</h1>;"
`;
exports[`ReactRefreshLogBox app default should strip whitespace correctly with newline 1`] = `
"index.js (8:26) @ onClick
6 |
7 | <a onClick={() => {
> 8 | throw new Error('idk')
| ^
9 | }}>
10 | click me
11 | </a>"
"index.js (7:14) @ onClick
5 |
6 | <a onClick={() => {
> 7 | throw new Error('idk')
| ^
8 | }}>
9 | click me
10 | </a>"
`;
exports[`ReactRefreshLogBox app turbo Can't resolve @import in CSS file 1`] = `
Expand All @@ -121,10 +121,10 @@ exports[`ReactRefreshLogBox app turbo Should not show __webpack_exports__ when e
"index.js (1:13) @ __TURBOPACK__default__export__
> 1 | export default () => {
| ^
2 | if (typeof window !== 'undefined') {
3 | throw new Error('test')
4 | }"
| ^
2 | if (typeof window !== 'undefined') {
3 | throw new Error('test')
4 | }"
`;
exports[`ReactRefreshLogBox app turbo boundaries 1`] = `
Expand Down Expand Up @@ -155,20 +155,20 @@ exports[`ReactRefreshLogBox app turbo module init error not shown 1`] = `
1 | // top offset for snapshot
2 | import * as React from 'react';
> 3 | throw new Error('no')
| ^
| ^
4 | class ClassDefault extends React.Component {
5 | render() {
6 | return <h1>Default Export</h1>;"
5 | render() {
6 | return <h1>Default Export</h1>;"
`;
exports[`ReactRefreshLogBox app turbo should strip whitespace correctly with newline 1`] = `
"index.js (6:30) @ onClick
"index.js (5:18) @ onClick
4 |
5 | <p>index page</p>
> 6 |
3 | <>
4 | <p>index page</p>
> 5 |
| ^
7 | <a onClick={() => {
8 | throw new Error('idk')
9 | }}>"
6 | <a onClick={() => {
7 | throw new Error('idk')
8 | }}>"
`;
14 changes: 7 additions & 7 deletions test/development/acceptance-app/error-recovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,13 @@ describe.each(['default', 'turbo'])('Error recovery app %s', () => {
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"index.js (7:10) @ eval
5 | const increment = useCallback(() => {
6 | setCount(c => c + 1)
> 7 | throw new Error('oops')
| ^
8 | }, [setCount])
9 | return (
10 | <main>"
5 | const increment = useCallback(() => {
6 | setCount(c => c + 1)
> 7 | throw new Error('oops')
| ^
8 | }, [setCount])
9 | return (
10 | <main>"
`)

await session.patch(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ exports[`ReactRefreshLogBox default conversion to class component (1) 1`] = `
"Child.js (4:10) @ ClickCount.render
2 | export default class ClickCount extends Component {
3 | render() {
> 4 | throw new Error()
| ^
5 | }
3 | render() {
> 4 | throw new Error()
| ^
5 | }
6 | }"
`;

Expand Down Expand Up @@ -52,22 +52,22 @@ exports[`ReactRefreshLogBox default module init error not shown 1`] = `
1 | // top offset for snapshot
2 | import * as React from 'react';
> 3 | throw new Error('no')
| ^
| ^
4 | class ClassDefault extends React.Component {
5 | render() {
6 | return <h1>Default Export</h1>;"
5 | render() {
6 | return <h1>Default Export</h1>;"
`;
exports[`ReactRefreshLogBox default should strip whitespace correctly with newline 1`] = `
"index.js (8:26) @ onClick
6 |
7 | <a onClick={() => {
> 8 | throw new Error('idk')
| ^
9 | }}>
10 | click me
11 | </a>"
7 | <a onClick={() => {
> 8 | throw new Error('idk')
| ^
9 | }}>
10 | click me
11 | </a>"
`;
exports[`ReactRefreshLogBox turbo boundaries 1`] = `null`;
Expand Down Expand Up @@ -98,20 +98,20 @@ exports[`ReactRefreshLogBox turbo module init error not shown 1`] = `
1 | // top offset for snapshot
2 | import * as React from 'react';
> 3 | throw new Error('no')
| ^
| ^
4 | class ClassDefault extends React.Component {
5 | render() {
6 | return <h1>Default Export</h1>;"
5 | render() {
6 | return <h1>Default Export</h1>;"
`;
exports[`ReactRefreshLogBox turbo should strip whitespace correctly with newline 1`] = `
"index.js (8:18) @ onClick
6 |
7 | <a onClick={() => {
> 8 | throw new Error('idk')
|^
9 | }}>
10 | click me
11 | </a>"
7 | <a onClick={() => {
> 8 | throw new Error('idk')
| ^
9 | }}>
10 | click me
11 | </a>"
`;
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ exports[`ReactRefreshLogBox default syntax > runtime error 1`] = `
"index.js (5:8) @ Error
3 | setInterval(() => {
4 | i++
> 5 | throw Error('no ' + i)
| ^
4 | i++
> 5 | throw Error('no ' + i)
| ^
6 | }, 1000)
7 | export default function FunctionNamed() {
8 | return <div />"
8 | return <div />"
`;

0 comments on commit 54f3399

Please sign in to comment.