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

Improve source map handling #699

Closed
7 of 8 tasks
marvinhagemeister opened this issue Jun 21, 2021 · 6 comments · Fixed by #715
Closed
7 of 8 tasks

Improve source map handling #699

marvinhagemeister opened this issue Jun 21, 2021 · 6 comments · Fixed by #715
Labels
bug Something isn't working

Comments

@marvinhagemeister
Copy link
Member

marvinhagemeister commented Jun 21, 2021

The source map handling in WMR can be a lot better. Namely:

  • Ensure --sourcemap generates sourcemaps Add groundwork for source map support #715
  • Merge source maps in container.transform()
  • Ensure acorn-traverse generates correct mappings
  • Always include sourcesContent, so that the original code is shown in browser devtools
  • Ensure npm plugin supports source maps (might need a rewrite) - out of scope for now. The rewrite will happen either way and we can do the source maps in one go
  • Add source map support to .sass (iirc they have different file mappings)
  • Decide on path format ./foo.js vs foo.js
  • Improve performance of source map generation (check if it can be made so fast, that we can turn this always on)
@marvinhagemeister marvinhagemeister added the bug Something isn't working label Jun 21, 2021
@Inviz
Copy link
Contributor

Inviz commented Jun 28, 2021

Huge plus one on this. Currently debugging things in builds is a pretty sizable PITA. On my app, chrome code formatted just gives up, and Sentry error reports also are pretty damn useless.

@Inviz
Copy link
Contributor

Inviz commented Jun 28, 2021

In my app what i'm seeing is that .js file has a sourcemap file that goes like this:

{"version":3,"file":"index.31a59432.js","sources":["source:///components/formField/style.module.scss","source:///components/combobox/style.module.scss","source:///components/form/style.module.scss","source:///components/button/style.module.scss"....

The issue is that sources array only has scss files, and no javascript files at all.

@Inviz
Copy link
Contributor

Inviz commented Jun 28, 2021

And there's these messages as well:

Sourcemap is likely to be incorrect: a plugin (default-loaders) was used to transform files, but didn't generate a sourcemap for the transformation.```

@Inviz
Copy link
Contributor

Inviz commented Jun 28, 2021

If defaultLoaders plugin is commented out from wmr, all sourcemaps start working

@marvinhagemeister
Copy link
Member Author

Had a go at this over the weekend and there are various places which are not aware of sourcemaps yet. Those generate wrong line and column mappings or miss source information completely. The default-laoders plugin is one of those places. Then the other question is about how to merge all the maps from the generated stages. This part is a little trickier and something other tools seem to have trouble with too.

One reason for column mismatches seems to be that something in our stack interprets column mappings wrong when a new line begins. The spec clearly says that all column positions of the original file are relative to the previous one, but something resets it to 0 with each new line.

Also found various ways to speed up source map generation and parsing in itself by eliminating all cases of allocations. The underlying library of magic-string that does the soucemapping puts a lot of pressure on the GC.

Looking forward to dive deeper into this!

@Inviz
Copy link
Contributor

Inviz commented Jun 29, 2021

Sounds super exciting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants