Skip to content
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.

Sign up for GitHub

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

Reapply "Turbopack: convert between locations correctly (#61477)" (#61733) #61735

Merged
merged 3 commits into from
Feb 6, 2024
Merged

Reapply "Turbopack: convert between locations correctly (#61477)" (#61733) #61735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
633ca3c
Reapply "Turbopack: convert between locations correctly (#61477)" (#6…
wbinnssmith Feb 6, 2024
93c708f
fixup! Reapply "Turbopack: convert between locations correctly (#6147…
wbinnssmith Feb 6, 2024
09f1265
fixup! Reapply "Turbopack: convert between locations correctly (#6147…
wbinnssmith Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions packages/next-swc/crates/napi/src/next_api/project.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,9 @@ pub fn project_update_info_subscribe(
#[derive(Debug)]
#[napi(object)]
pub struct StackFrame {
// 1-indexed, unlike source map tokens
pub column: Option<u32>,
// 1-indexed, unlike source map tokens
pub file: String,
pub is_server: bool,
pub line: u32,
Expand Down Expand Up @@ -876,7 +878,10 @@ pub async fn project_trace_source(
};

let token = map
.lookup_token(frame.line as usize, frame.column.unwrap_or(0) as usize)
.lookup_token(
(frame.line as usize).saturating_sub(1),
(frame.column.unwrap_or(1) as usize).saturating_sub(1),
)
.await?
.clone_value()
.context("Unable to trace token from sourcemap")?;
Expand All @@ -893,8 +898,8 @@ pub async fn project_trace_source(
Ok(Some(StackFrame {
file: source_file.to_string(),
method_name: token.name,
line: token.original_line as u32,
column: Some(token.original_column as u32),
line: token.original_line as u32 + 1,
column: Some(token.original_column as u32 + 1),
is_server: frame.is_server,
}))
})
Expand Down
14 changes: 12 additions & 2 deletions packages/next/src/build/swc/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,18 @@ export interface Issue {
content?: string
}
range?: {
start: { line: number; column: number }
end: { line: number; column: number }
start: {
// 0-indexed
line: number
// 0-indexed
column: number
}
end: {
// 0-indexed
line: number
// 0-indexed
column: number
}
}
}
documentationLink: string
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/lib/router-utils/setup-dev-bundler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ async function startWatcher(opts: SetupOpts) {
if (source && source.range) {
const { start } = source.range
message = `${formattedFilePath}:${start.line + 1}:${
start.column
start.column + 1
}\n${formattedTitle}`
} else if (formattedFilePath) {
message = `${formattedFilePath}\n${formattedTitle}`
Expand Down
4 changes: 3 additions & 1 deletion packages/react-dev-overlay/src/middleware-turbopack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ interface Project {
}

interface TurbopackStackFrame {
// 1-based
column: number | null
// 1-based
file: string
isServer: boolean
line: number
Expand Down Expand Up @@ -87,7 +89,7 @@ export async function createOriginalStackFrame(
{
start: {
line: traced.frame.lineNumber,
column: traced.frame.column ?? 0,
column: traced.frame.column ?? 1,
},
},
{ forceColor: true }
Expand Down
5 changes: 3 additions & 2 deletions packages/react-dev-overlay/src/middleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ export async function createOriginalStackFrame({

return moduleNotFoundResult
}
// This returns 1-based lines and 0-based columns
return await findOriginalSourcePositionAndContent(source, {
line,
column,
Expand Down Expand Up @@ -225,7 +226,7 @@ export async function createOriginalStackFrame({
? path.relative(rootDirectory, filePath)
: sourcePosition.source,
lineNumber: sourcePosition.line,
column: sourcePosition.column,
column: (sourcePosition.column ?? 0) + 1,
methodName:
sourcePosition.name ||
// default is not a valid identifier in JS so webpack uses a custom variable when it's an unnamed default export
Expand All @@ -245,7 +246,7 @@ export async function createOriginalStackFrame({
{
start: {
line: sourcePosition.line,
column: sourcePosition.column ?? 0,
column: (sourcePosition.column ?? 0) + 1,
},
},
{ forceColor: true }
Expand Down
19 changes: 11 additions & 8 deletions test/development/acceptance-app/ReactRefreshLogBox-builtins.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
expect(await session.hasRedbox()).toBe(true)
if (process.env.TURBOPACK) {
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"./node_modules/my-package/index.js:1:12
"./node_modules/my-package/index.js:1:13
Module not found: Can't resolve 'dns'
> 1 | const dns = require('dns')
| ^^^^^^^^^^^^^^
Expand All @@ -62,7 +62,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
`)
} else {
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"./node_modules/my-package/index.js:1:12
"./node_modules/my-package/index.js:1:13
Module not found: Can't resolve 'dns'
https://nextjs.org/docs/messages/module-not-found
Expand Down Expand Up @@ -98,7 +98,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
const source = await session.getRedboxSource()
if (process.env.TURBOPACK) {
expect(source).toMatchInlineSnapshot(`
"./index.js:1:0
"./index.js:1:1
Module not found: Can't resolve 'b'
> 1 | import Comp from 'b'
| ^^^^^^^^^^^^^^^^^^^^
Expand All @@ -110,9 +110,10 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
`)
} else {
expect(source).toMatchInlineSnapshot(`
"./index.js:1:0
"./index.js:1:1
Module not found: Can't resolve 'b'
> 1 | import Comp from 'b'
| ^
2 | export default function Oops() {
3 | return (
4 | <div>
Expand Down Expand Up @@ -150,7 +151,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
const source = await session.getRedboxSource()
if (process.env.TURBOPACK) {
expect(source).toMatchInlineSnapshot(`
"./app/page.js:2:0
"./app/page.js:2:1
Module not found: Can't resolve 'b'
1 | 'use client'
> 2 | import Comp from 'b'
Expand All @@ -163,10 +164,11 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
`)
} else {
expect(source).toMatchInlineSnapshot(`
"./app/page.js:2:0
"./app/page.js:2:1
Module not found: Can't resolve 'b'
1 | 'use client'
> 2 | import Comp from 'b'
| ^
3 | export default function Oops() {
4 | return (
5 | <div>
Expand Down Expand Up @@ -199,7 +201,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
const source = await session.getRedboxSource()
if (process.env.TURBOPACK) {
expect(source).toMatchInlineSnapshot(`
"./app/page.js:2:0
"./app/page.js:2:1
Module not found: Can't resolve './non-existent.css'
1 | 'use client'
> 2 | import './non-existent.css'
Expand All @@ -212,10 +214,11 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
`)
} else {
expect(source).toMatchInlineSnapshot(`
"./app/page.js:2:0
"./app/page.js:2:1
Module not found: Can't resolve './non-existent.css'
1 | 'use client'
> 2 | import './non-existent.css'
| ^
3 | export default function Page(props) {
4 | return <p>index page</p>
5 | }
Expand Down
4 changes: 2 additions & 2 deletions test/development/acceptance-app/ReactRefreshLogBox.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
const source = next.normalizeTestDirContent(await session.getRedboxSource())
if (IS_TURBOPACK) {
expect(source).toMatchInlineSnapshot(`
"./index.js:7:0
"./index.js:7:1
Parsing ecmascript source code failed
5 | div
6 | )
Expand Down Expand Up @@ -357,7 +357,7 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => {
expect(await session.hasRedbox()).toBe(true)
const source = await session.getRedboxSource()
expect(source).toMatch(
IS_TURBOPACK ? './index.module.css:1:8' : './index.module.css:1:1'
IS_TURBOPACK ? './index.module.css:1:9' : './index.module.css:1:1'
)
if (!IS_TURBOPACK) {
expect(source).toMatch('Syntax error: ')
Expand Down
12 changes: 6 additions & 6 deletions test/development/acceptance-app/ReactRefreshRegression.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ describe('ReactRefreshRegression app', () => {
const source = await session.getRedboxSource()
expect(source.split(/\r?\n/g).slice(2).join('\n')).toMatchInlineSnapshot(`
"> 1 | export default function () { throw new Error('boom'); }
| ^"
| ^"
`)

await cleanup()
Expand All @@ -305,7 +305,7 @@ describe('ReactRefreshRegression app', () => {
const source = await session.getRedboxSource()
expect(source.split(/\r?\n/g).slice(2).join('\n')).toMatchInlineSnapshot(`
"> 1 | export default function Page() { throw new Error('boom'); }
| ^"
| ^"
`)

await cleanup()
Expand All @@ -326,10 +326,10 @@ describe('ReactRefreshRegression app', () => {

const source = await session.getRedboxSource()
expect(source.split(/\r?\n/g).slice(2).join('\n')).toMatchInlineSnapshot(`
" 1 | 'use client'
> 2 | export default function Page() { throw new Error('boom'); }
| ^"
`)
" 1 | 'use client'
> 2 | export default function Page() { throw new Error('boom'); }
| ^"
`)

await cleanup()
})
Expand Down
44 changes: 23 additions & 21 deletions test/development/acceptance-app/__snapshots__/ReactRefreshLogBox.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ Import trace for requested module:
`;

exports[`ReactRefreshLogBox app default Import trace when module not found in layout 1`] = `
"./app/module.js:1:0
"./app/module.js:1:1
Module not found: Can't resolve 'non-existing-module'
> 1 | import "non-existing-module"
| ^
https://nextjs.org/docs/messages/module-not-found
Expand All @@ -22,31 +23,31 @@ Import trace for requested module:
`;

exports[`ReactRefreshLogBox app default Should not show __webpack_exports__ when exporting anonymous arrow function 1`] = `
"index.js (3:10) @ default
"index.js (3:11) @ default
1 | export default () => {
2 | if (typeof window !== 'undefined') {
> 3 | throw new Error('test')
| ^
| ^
4 | }
5 |
6 | return null"
`;
exports[`ReactRefreshLogBox app default boundaries 1`] = `
"FunctionDefault.js (1:50) @ FunctionDefault
"FunctionDefault.js (1:51) @ FunctionDefault
> 1 | export default function FunctionDefault() { throw new Error('no'); }
| ^"
| ^"
`;
exports[`ReactRefreshLogBox app default conversion to class component (1) 1`] = `
"Child.js (4:10) @ ClickCount.render
"Child.js (4:11) @ ClickCount.render
2 | export default class ClickCount extends Component {
3 | render() {
> 4 | throw new Error()
| ^
| ^
5 | }
6 | }"
`;
Expand Down Expand Up @@ -76,31 +77,31 @@ exports[`ReactRefreshLogBox app default logbox: anchors links in error messages
exports[`ReactRefreshLogBox app default logbox: anchors links in error messages 9`] = `"http://example.com/"`;
exports[`ReactRefreshLogBox app default module init error not shown 1`] = `
"index.js (3:6) @ eval
"index.js (3:7) @ eval
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>;"
`;
exports[`ReactRefreshLogBox app default should strip whitespace correctly with newline 1`] = `
"index.js (7:14) @ onClick
"index.js (7:15) @ 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`] = `
"./app/styles1.css:1:0
"./app/styles1.css:1:1
Parsing css source code failed
> 1 | @import "./styles2.css"
| ^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -109,7 +110,7 @@ Unexpected end of file, but expected ';' or '{'"
`;
exports[`ReactRefreshLogBox app turbo Import trace when module not found in layout 1`] = `
"./app/module.js:1:0
"./app/module.js:1:1
Module not found: Can't resolve 'non-existing-module'
> 1 | import "non-existing-module"
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -118,21 +119,22 @@ https://nextjs.org/docs/messages/module-not-found"
`;
exports[`ReactRefreshLogBox app turbo Should not show __webpack_exports__ when exporting anonymous arrow function 1`] = `
"index.js (3:2) @ {default export}
"index.js (3:11) @ {default export}
1 | export default () => {
2 | if (typeof window !== 'undefined') {
> 3 | throw new Error('test')
| ^
| ^
4 | }
5 |
6 | return null"
`;
exports[`ReactRefreshLogBox app turbo boundaries 1`] = `
"FunctionDefault.js (0:67) @ FunctionDefault
"FunctionDefault.js (1:51) @ FunctionDefault
1 | export default function FunctionDefault() { throw new Error('no'); }"
> 1 | export default function FunctionDefault() { throw new Error('no'); }
| ^"
`;
exports[`ReactRefreshLogBox app turbo logbox: anchors links in error messages 1`] = `"Error: end http://nextjs.org"`;
Expand All @@ -152,24 +154,24 @@ exports[`ReactRefreshLogBox app turbo logbox: anchors links in error messages 8`
exports[`ReactRefreshLogBox app turbo logbox: anchors links in error messages 9`] = `"http://example.com/"`;
exports[`ReactRefreshLogBox app turbo module init error not shown 1`] = `
"index.js (3:6) @ <unknown>
"index.js (3:7) @ <unknown>
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>;"
`;
exports[`ReactRefreshLogBox app turbo should strip whitespace correctly with newline 1`] = `
"index.js (7:6) @ onClick
"index.js (7:15) @ onClick
5 |
6 | <a onClick={() => {
> 7 | throw new Error('idk')
| ^
| ^
8 | }}>
9 | click me
10 | </a>"
Expand Down
Loading
Loading