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.

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

Inaccurate sourcemaps for sources under node_modules #53253

Open
1 task done
hbenl opened this issue Jul 27, 2023 · 15 comments
Open
1 task done

Inaccurate sourcemaps for sources under node_modules #53253

hbenl opened this issue Jul 27, 2023 · 15 comments
Labels
bug Issue was opened via the bug report template.

Comments

@hbenl
Copy link

hbenl commented Jul 27, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: linux
  Arch: x64
  Version: #85~20.04.1-Ubuntu SMP Mon Jul 17 09:42:39 UTC 2023
Binaries:
  Node: 18.16.0
  npm: 9.5.1
  Yarn: 1.22.18
  pnpm: 8.6.7
Relevant Packages:
  next: 13.4.12
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: N/A
Next.js Config:
  output: N/A

Which area(s) of Next.js are affected? (leave empty if unsure)

No response

Link to the code that reproduces this issue or a replay of the bug

https://github.com/replayio/nextjs-inaccurate-sourcemaps

To Reproduce

Alternatively, you can look at these sourcemaps here:

Describe the Bug

I've noticed that the sourcemaps produced by Next.js are inaccurate for some sources, making it impossible for debuggers to show scopes with original (rather than generated) variable names.
This seems to be the case for all sources under node_modules.
To reproduce the problem I've used create-next-app to create an app and copied node_modules/@swc/helpers/esm/_interop_require_wildcard.js to src/_interop_require_wildcard.js. This is what a sourcemap visualizer shows for these two copies of the same source:

node_modules/@swc/helpers/esm/_interop_require_wildcard.js:

Screenshot_2023-07-27_13-49-26

src/_interop_require_wildcard.js:

Screenshot_2023-07-27_13-50-09

Expected Behavior

Sourcemaps for sources under node_modules should be accurate.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

@hbenl hbenl added the bug Issue was opened via the bug report template. label Jul 27, 2023
@leerob
Copy link
Member

leerob commented Aug 23, 2023

Have you determined what version this started to be an issue for you?

@jridgewell
Copy link
Contributor

This seems like it's defaulting to a low fidelity source map for inputs which don't have a source map. It affects node_modules/@swc/helpers/…, but not next/dist/compiled/… (which has an source map to the TS sources). I think it's likely that we're not transforming the source (because it's inside node_modules), so we don't generate a high-fidelity AST mapping.

@hbenl
Copy link
Author

hbenl commented Aug 24, 2023

Have you determined what version this started to be an issue for you?

No, I don't know if we ever got better sourcemaps - we rarely debug sources under node_modules (because it's mostly third-party code) but recently we had a few issues where we wanted to understand the behavior of some code under node_modules and that's when we noticed the issue.

I also wanted to highlight something peculiar: while in the screenshots above it looks like the inaccurate sourcemap is (mostly) using one mapping per line, it actually uses the same number of mappings per line as the accurate sourcemap - but the mappings all point to column 0 in the original source (have a look here and here).
Initially I thought that maybe Next.js intentionally creates "low fidelity sourcemaps" for code under node_modules to reduce the overall size of the sourcemaps but since those sourcemaps have the same number of mappings the size won't be reduced (at least not significantly).

@bvaughn
Copy link

bvaughn commented Aug 24, 2023

but since those sourcemaps have the same number of mappings the size won't be reduced (at least not significantly).

I wonder if it's less a concern about size and more a concern about build times?

@markerikson
Copy link

I'm seeing pretty similar issues myself (ironic given that I work with Holger and Brian!).

Per markerikson/react-prod-sourcemaps#4 (comment) , I have created a package that contains backported sourcemaps for existing versions of ReactDOM.

I can view one of my sourcemaps by itself, and see that each individual function argument is mapped correctly.

However, when I added the build plugin to swap the ReactDOM sourcemap to our own Next + Webpack config, the results show that the entire function declaration line is getting mapped as a single block, rather than separate mappings for the function name and each separate argument.

@markerikson
Copy link

React just merged my PR to start generating sourcemaps:

and I see that Next has updated to pull in that build of React:

But, as long as this issue exists, the React sourcemaps in Next will likely be semi-broken because function arguments are losing their mappings.

@jridgewell
Copy link
Contributor

If React is now publishing the source maps as part of the NPM module, then all we would need to do is hook it up so that input code could reference source maps, and feed that through to the output assets. I think the only line that needs to be changed is https://github.com/vercel/turbo/blob/fffcfd86baf9f150f7d443e96f4d18733311352f/crates/turbopack-ecmascript/src/lib.rs#L693. We'll need to check if there's a sourceMappingURL in the referenced source file and link the source map, but that should be easy.

@hbenl
Copy link
Author

hbenl commented Nov 10, 2023

@jridgewell If I understand your comment correctly, this would include the sourcemaps for React but it wouldn't change the fact that those sourcemaps will be semi-broken (which is what this issue is about).

@jridgewell
Copy link
Contributor

I tried reproducing this with latest 14.0.2, and I can't find a broken _interop_require_wildcard in the maps. At some point, we started generating full maps for all inputs (regardless of if they're in node_modules). We still don't use an inputs own map.

@hbenl
Copy link
Author

hbenl commented Nov 16, 2023

I also tried with 14.0.2 - here is the branch and here is a recording and this link shows that _interop_require_wildcard.js still has an inaccurate sourcemap.

@jridgewell
Copy link
Contributor

Oh, this is using webpack? I thought this entire thread was about Turbopack. 😅

@bvaughn
Copy link

bvaughn commented Feb 5, 2024

This issue continues to be a pain point for us, @leerob. Any chance someone at Vercel could take a look? If you need any more info from us, we'll be happy to provide it.

@jasonLaster
Copy link

@jridgewell any chance you'd be open to backporting a similar fix to Webpack? I think at this point we should also try out Turbo, but either way it'd be great if folks in the Webpack community benefited from React source maps as well.

@jridgewell
Copy link
Contributor

Redirecting to @sokra

@mauron85
Copy link

mauron85 commented Aug 19, 2024

Joining the discussion. Using source-map-loader or (smart-source-map-loader) some internal nextjs sourcemaps are not having correct urls.

For instance client/router.js can be located in chrome devtools under:

webpack-internal://node_modules/next/dist/client/router.js

But when I click to setup breakpoint I'm redirect to file:

webpack://_N_E/src/client/router.ts?6d93

And instead of source I see error:

Could not load content for webpack://_N_E/src/client/router.ts?6d93 (Fetch through target failed: Unsupported URL scheme; Fallback: HTTP error: status code 404, net::ERR_UNKNOWN_URL_SCHEME)

In the browser console:

hot-reloader-client.ts:167  ./node_modules/next/dist/client/add-base-path.js
smart-source-map-loader: Failed to resolve source file: add-base-path.js.map -> ./../../src/client/add-base-path.ts: Error: Can't resolve './../../src/client/add-base-path.ts' in 'C:\Colonnade\travel_portal_fe\node_modules\next\dist\client'

Import trace for requested module:
./node_modules/next/dist/client/add-base-path.js
./node_modules/next/dist/shared/lib/router/router.js

So it seems sourcemaps are not generated correctly, by using reference to non present src folder with .ts .tsx files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

No branches or pull requests

7 participants