Missing/broken sourcemaps for JS modules w/ imports when used with Vue

Smef commented 2 years ago

Describe the bug

I originally reported this to JetBrains, and they seem to think it's an issue caused by sourcemaps not being generated by Vite. The same project moved over, running, and debugging on Vue-CLI does not have this issue and breakpoints are hit in the correct file.

https://youtrack.jetbrains.com/issue/WEB-55544

IntelliJ/WebStorm/VSCode seems to be unable to properly debug local JavaScript files when debugging JS modules imported into Vue components in a Vite project.

If you import a regular JS module into your component which itself has an import IJ/WS/VSC is unable to properly map the local .js file to the remote file for JavaScript debugging purposes due to missing/broken sourcemaps. When you place a breakpoint in your JS module source a new, read-only copy of the remote file is loaded from http://localhost:3000/src/xxx/xxx.js in the IDE and the breakpoint stops there instead of in the original JS file. This means that you have to debug in a read-only copy and then swap to your local copy for edits, which can be a big pain.

It looks like this only happens when the modules themselves have imports. You can see that the remote file has a changed "import" line and so that might be causing it to not match up. Here's a screenshot from a sample project where the import inside the module causes the mismatch. You can see this difference between the local and "remote" on line 1.

I've also created an example project which replicates this issue.

https://github.com/Smef/vite-debug-issue-demo

Reproduction

https://github.com/Smef/vite-debug-issue-demo

System Info

System:
    OS: macOS 12.3.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 147.20 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.14.0 - ~/.nvm/versions/node/v16.14.0/bin/node
    npm: 8.3.1 - ~/.nvm/versions/node/v16.14.0/bin/npm
  Browsers:
    Chrome: 100.0.4896.88
    Firefox: 99.0.1
    Safari: 15.4
  npmPackages:
    @vitejs/plugin-vue: ^2.3.1 => 2.3.1 
    vite: ^2.9.5 => 2.9.5

Used Package Manager

npm

Logs

No response

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to https://github.com/vuejs/core instead.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

sapphi-red commented 2 years ago

duplicate of vitejs/vite-plugin-vue#33

Smef commented 2 years ago

This doesn’t sound like the same issue to me ( but maybe it shares an underlying cause?). This doesn’t have anything to do with reloading and isn’t something that affects Vue components. This only affects vanilla JS modules which import other JS modules.

Line numbers aren’t messed up, and the issue is not breaking on an incorrect line, but instead an incorrect file.

I’ve seen the HMR reload issue myself, and I actually encountered it when trying to create a simple example of this issue, but this is not the same thing.

Smef commented 2 years ago

And to clarify, this is something that happens 100% of the time and still persists after getting past the HMR issue discussed in the other issue.

sapphi-red commented 2 years ago

I thought they were the same, but it seem to be slightly different, so I will reopen this one. Thank you.

BenceSzalai commented 2 years ago

I am facing the same issue. It is kind of annoying, which is not a valuable addition to the discussion, I know, but I'm saying this to offer any additional information or help for the issue, as it makes debugging quite miserable.

BenceSzalai commented 2 years ago

Looking into it looks like to me the issue is caused by simply the fact that the file seen by the browser and by the IDE are different. Probably there's a hash or byte-wise comparison, and if they don't match, the IDE shows the actual file, not the one in the project.

So for example if I have a module that has an import in line 1 such as:

import { useAuthStore } from 'stores/auth'

Vite will change this line by the time it gets in the browser to:

import { useAuthStore } from '/src/stores/auth.js'

therefore the IDE sees it as a different file.

At this point I'd like to say that if this really is the reason, the IDE does it right, as if the file is different in the browser, proper debugging is not possible, unless the same file is being shown in the debugger.

To confirm this consider the following. My project is set up in a way, that my application compiles just as well if I replace the given import in line 1, with the same that Vite would send to the browser:

import { useAuthStore } from '/src/stores/auth.js'

At this point there is no longer any difference between the file known by the IDE and the browser, they match byte-to-byte. And voilà, the debugger in IDE now opens the source file from the project, instead of the browser.

Based on this I would state cause of the issue is that Vite changes the files sent to the browser thus prevents the IDE to allow a debugger to step over the original source file.

Now this is an assumption of mine, but based on the comment from JetBrains a proper sourcemap could solve the issue. My understanding is that if a sourcemap was provided, the browser could use that to mock the original source file instead of the actual file it has received. If the sourcemap works correctly, the mocked file would match byte-to-byte the original file in the project, so the IDE could allow the stepper to work with the original source file instead of showing the modified file from the browser.

For me the solution seems to be to add source map to all files that are changed, even if not minified.

I've tried to enable source mapping in dev mode, but could not. I've even tried to enable minification, as even the overhead to minify would worth it just to get proper debugging - assuming that if minified, sourcemap would be added - but I couldn't. Not sure if that is not supported by vite or my build config.

Anyway, the goal is to get sourcemaps for JS files in dev mode. Anyone has a pointer to where to look forward, please?

BenceSzalai commented 2 years ago

So after some digging it looks like to me, that buildImportAnalysisPlugin does generates source map, while importAnalysisPlugin does not. So probably the way to fix it is to extend the later with features of the former.

BenceSzalai commented 2 years ago

I've got it working by implementing source maps inside importAnalysisPlugin.

I've been working on v2.9.13, so the changes are not directly applicable to the latest version of the code, but the point is that in https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugins/importAnalysis.ts#L725 it should not only return the source code but generate a sourceMap as well.

In v2.9.13 it can simply be done by replacing the line:

if (s) {
    return s.toString();
}

with:

if (s) {
    return { code: s.toString(), map: s.generateMap({ hires: false, source: importerModule.url, includeContent: true }) }
}

The same logic can be applied in the latest branch, by modifying https://github.com/vitejs/vite/blob/main/packages/vite/src/node/utils.ts#L1114 which righ now only generates source map in build mode.

However that is an oversight as without proper SourceMaps debugging cannot be used, which should be considered in dev mode.

Probably the best would be to simply add a new option under server, e.g.: config.server.sourcemap and if set to true, generate them. This way anyone willing to debug using the IDE could simply accept the slight performance penalty, but still generate the maps.

Let me know your thoughts also if a PR would be welcome for these changes. I guess documentation would need to be updated as well, but after all it is a pretty low key change to fix such an annoying and long standing issue!

Meanwhile anyone can monkey-patch their vite following the above and applying the same changes in the dist chunks in node_modules. In v2.9.13 the lines to be changed can be found under node_modules/vite/dist/node/chunks/dep-80fe9c6b.js from line 58644 (or the same line if using Quasar in node_modules/@quasar/app-vite/node_modules/vite/dist/node/chunks/dep-80fe9c6b.js.

sapphi-red commented 2 years ago

Thanks for digging into this one. I am thinking that it might only matter whether the sourcemap maps foo.js?t=bar to foo.js. Would you check if this works?

if (s) {
    const newCode = s.toString()
    const map = new MagicString(newCode).generateMap({ source: importerModule.url });
    return { code: s.toString(), map }
}

If this works, I think we only need to inject sourcemap when a query is injected (example: ?t=) and sourcemap is not generated by anything.

BenceSzalai commented 2 years ago

Afaics only adding the source without the proper mappings may only work as long as the number of lines are not changed and there are no changes at all to actual code lines (i mean other than imports). Also I think this is exactly the case here, so it may be a nice trick to reduce overhead. Will try soon an update.

BenceSzalai commented 2 years ago

I can confirm it is working at least in my test case as shown above, with only setting the source in the map, without generating any mappings.

By the same token, if the overhead associated with generating an actual SourceMap is not welcome in dev mode (understandable), maybe importAnalysisPlugin should use just a regular string instead of a MagicString, as why to bear the overhead of collecting the changes if we don't use them at the end anyway? (I don't know actually how the overhead of using MagicString to manipulate the source vs. generating the proper map that includes mappings compares. It might be the case that the former is negligible overhead, but I don't know. Was it tested/examined, or it may only be an illusion that not generating a map at the end solves the overhead, if prepend(), overwrite() etc calls to MagicString are also significantly slower than just splitting and concatenating standard srings. Not sure...)

BenceSzalai commented 2 years ago

If this works, I think we only need to inject sourcemap when a query is injected (example: ?t=) and sourcemap is not generated by anything.

Sourcemap always have to be injected when there is a change made to any JS files, because otherwise the IDE cannot properly present the debugger. The initial request from the browser for a JS file imported into a Vue component (my test case) is without any query string. Of course, if I make a change and hot reload gets a new version of the file, a ?t=... query will be attached, but mapping to the original source file is needed also for the initially loaded file.

So the cases I see are:

1.) Contents of a file are not changed at all by Vite, just served as they are: no need for sourcemap, because the IDE will know based on path the correct file, and when the IDE compares the file seen by the Browser to the file it has will recognise they are the same - it is an assumption on my side, but this explains the behaviour I see, namely that if the file is edited in a way, that Vite does not have to make changes to it, the debugger works fine even without a sourcemap.

2.) Contents of a file are changed by Vite, but the changes are only rewritten imports: no need for an actual sourcemap, but needs a dummy sourcemap with an empty mappings, names etc, only having a one element sources array pointing at the original file. This will only work properly as long as no new lines are inserted or no lines are removed from the file. Here we can utilise the fact that the imports are fine to be different in the IDE and the browser as an import cannot be stepped into, and no other lines of the code are changed therefore when the IDE steps in the code it can show the proper line and position.

3.) Contents of a file are changed by Vite beyond only rewritten imports: In this case a full blown sourcemap is required for proper debugging. Without a sourcemap the IDE would show a read only copy of the served file (original issue) and if a minimal sourcemap is used as in 2. the IDE would show breakpoints at incorrect lines and character positions.

Now I'm not sure if Vite would ever do a type 3. change to a file during serving it in dev mode. As long as it is only 1. or 2. and we add the minimalistic sourcemap with only a single element source array pointing to the original source file, it should be okay. But in this case MagicString is not needed at all, simple string manipulation could be sufficient.

sapphi-red commented 2 years ago

By the same token, if the overhead associated with generating an actual SourceMap is not welcome in dev mode (understandable), maybe importAnalysisPlugin should use just a regular string instead of a MagicString, as why to bear the overhead of collecting the changes if we don't use them at the end anyway?

Generating the sourcemap is the most heavy operation among MagicString related ones. Creating the MagicString instance and doing manipulation by it does not have a large impact on performanace. (Actually creating an instance is a bit heavy but it should be solved by https://github.com/Rich-Harris/magic-string/pull/216 after the new release)

So the cases I see are:

I guess injecting a dummy sourcemap won't have much impact on performance. So I think we could simply inject a dummy sourcemap here if a sourcemap was not injected. (If a plugin changed the contents of the file and changing the lines, that plugin should be generating the sourcemap and that sourcemap will injected here.)

https://github.com/vitejs/vite/blob/aff4544d7ed85fd21e9c21abcb532dc72fe6540b/packages/vite/src/node/server/transformRequest.ts#L254-L259

Would you like to create a PR for this? If not, I will create it.

BenceSzalai commented 2 years ago

Meanwhile v3 has changed the affected code a bit factoring it into an utility method, so the PR has different bits, but the meaning is the same.

Also I've added the hires parameter for the simplified sourcemap too, as without it the resolution of the sourcemap of some existing test cases were reduced, probably because other manipulations on the file continue to accumulate changes in the same sourcemap, so it does matter to initialise it with hires: true.

The PR includes one new test in playground/js-sourcemap that shows the difference between the case of a JS file without imports (no sourcemap since nothing is changed) and one with import (needs sourcemap).

Hope it'll be okay like this.

selrond commented 1 year ago

I’m using VSCode & working with React and I face the same issue. It only manifests itself when I run the app (and try to debug it) from Docker, though, locally it’s fine. Does it make sense?

selrond commented 1 year ago

More findings—it seems to happen only with .js files, not .jsx nor .ts / .tsx

Enzojz commented 1 year ago

Same issue with VSCode + Vite + Vue3, happens with .js, not .vue

BenceSzalai commented 1 year ago

Yup, we are only talking about .js. Most other file types, like .ts have a correct sourceMap already.

The explanation for the Docker vs. Local difference can be that VSCode seem to be less picky about the correct sourceMap when it can match the files based on path, but it may not understand how to do a 3 way match, like the file in your IDE, the file in Docker and the file reported by the browser. But I'm only speculating here.

The PR is near merge I think, so it should be resolved soon. If you know how to monkey patch your node_modules, you can try with this patch and let me know if it solves your issues: https://github.com/BenceSzalai/vite/tree/fix-missing-serve-sourcemaps

The PR is here: https://github.com/vitejs/vite/pull/9476

Enzojz commented 1 year ago

Just installed 4.0-alpha-5 for preview, in vscode it stops at breakpoint at correct file, but when I remove the breakpoint they still stop until I close the attached browser.

BenceSzalai commented 1 year ago

@Enzojz: I'm not using VSCode myself, but I had similar case with PHPStorm. Afaics it happen when I set and unset breakpoints in both the browser (Chrome) and the IDE in the same session. When I only set and unset breakpoints in one, so either only in Browser or only in IDE it works fine for me. If I unset a breakpoint in the Browser which I configured in the IDE, it disappears, but still breaks execution, so that's not a good practice.

As a side-note: I think it is unrelated to this issue

On one hand because it only happens (at least for me) when HMR is actively being used and there are multiple versions of the same file loaded already in the same session. A full page refresh/reload cleans it up, so it is an HMR issue, if any, while this one is not really HMR related, but more general.
On the other hand the fix vitejs/vite#9476 is only available in 4.0.0-alpha.6, not 4.0.0-alpha.5 (according to the changelog)

Enzojz commented 1 year ago

@Enzojz: I'm not using VSCode myself, but I had similar case with PHPStorm. Afaics it happen when I set and unset breakpoints in both the browser (Chrome) and the IDE in the same session. When I only set and unset breakpoints in one, so either only in Browser or only in IDE it works fine for me. If I unset a breakpoint in the Browser which I configured in the IDE, it disappears, but still breaks execution, so that's not a good practice.

As a side-note: I think it is unrelated to this issue

On one hand because it only happens (at least for me) when HMR is actively being used and there are multiple versions of the same file loaded already in the same session. A full page refresh/reload cleans it up, so it is an HMR issue, if any, while this one is not really HMR related, but more general.

On the other hand the fix fix: missing js sourcemaps with rewritten imports broke debugging (#7767) #9476 is only available in 4.0.0-alpha.6, not 4.0.0-alpha.5 (according to the changelog)

I set the breakpoint only in vscode, it happens.
And refreshing the page doesn't resolve the problem. only closing the browser works.
a mistake, I was on alpha6

BenceSzalai commented 1 year ago

@Enzojz : Have you been using this kind of debugging in the same setup (same IDE, same project, same browser etc.) prior to v4.0.0-alpha.6 without issues?

BenceSzalai commented 1 year ago

@patak-dev Could you please reopen this issue, now that the related PR has been rolled back due to unexpected side effect in SvelteKit? (#11144)

I'll make a new PR with a different approach because the "fix" implemented in vitejs/vite#9476 was only a workaround, but looks like insufficient for all use cases.

BenceSzalai commented 1 year ago

So additional findings:

The propsed fix vitejs/vite#9476 broke tests in SvelteKit as can be seen here: https://github.com/sveltejs/kit/pull/7543
The propsed fix vitejs/vite#9476 has been unmerged
This issue is still an issue. (Should be reopened)
The problem is that by adding the dummy sourcemap some files which did not have any sourcemaps till now suddenly got one, however since the added sourcemaps were generated with the MagicString config highres: false it only included line numbers, which broke the tests mentioned above, because the test suit expected the columns to be correct as well. This is because that file in the test was not rewritten by Vite, so no sourcemap meant exact match was possible, but the non-high-res sourcemap made the mapping worse.
Obvious solution would be to implement high-res sourcemaps.
High res sourcemaps were not implemented in vitejs/vite#9476 because of assumptions about bad performance impact.
During investigation we also found that importAnalysis plugin tracks changes during dev in a MagicString instance, just skips the sourcemap generation at the end. This means it would be better to generate the sourcemap there instead of the workaround dummy sourcemap generation.

Now the plan is this (thx @dominikg): 1) revert PR vitejs/vite#9476 to restore the behaviour we know (see here: vitejs/vite#11144) 2) update the original issue with the additional information (see this comment) 3) start a new PR, add 2 testcases that a) check stacktrace positions with line and column and b) sourcemap accuracy in general. b would initially fail due to importanalysis updates not being recorded 4) implement proper sourcemap support for importanalysis in dev (test should pass now) 5) test perf impact by comparing main with that PR 6) if 5 shows low to no impact, just change the behavior, if it shows slowdowns, add an option

Regarding adding an option, obviously bloating the config with options is not a goal, but the current situation actually hurts both of 2 possible usecases:

Someone with high sensitivity to performance and willing to give up sourcemaps cannot do it, and most assets still include sourcemaps also MagicString is used by importAnalysis adding even more performance impact, even thought the map is not generated there. This all slows down their serve without any use.
Someone who needs the maps and willing to accept the performance penalty is almost good, except for JS assets, which is a huge blow for those working on non-TypeScript projects.

Obviously, the benefit of giving up sourcemaps would need more changes to transformation plugins, as all should than observe this configuration and skip using MagicString and generating sourcemaps, but it could potentially speed up dev mode a lot when sourcemaps are not needed.

So now I'll make another PR with a more comprehensive fix and do some tests there to see if performance is really a concern to address with a new setting, or can we just enable sourcemaps for JS importAnalysis and be happy with that.

EDIT: We have switched to TypeScript where this is basically a no issue, so I'll eventually not got to the point of working on it any further.

Enzojz commented 1 year ago

@Enzojz : Have you been using this kind of debugging in the same setup (same IDE, same project, same browser etc.) prior to v4.0.0-alpha.6 without issues?

Before alpha-6 the breakpoint in same .js file doesn't work at all as described (it goes into another browser side .js file with suffix and readonly)
yes, always same setting.

barbalex commented 1 year ago

Not sure if I have the same issue. I have migrated a gatsby app to vite. But left all js and jsx files as is. So there may be some thing to do with ts happening here.

When I publish the app (building it on vercel), then test in lighthouse, I get:

Could this have to do with the VERY big size of these two files?

JosefSaltz commented 1 year ago

I seem to be having this issue after migrating from CRA. I came here from Jetbrain Youtrack Issue. VScode hits breakpoints that return a remote copy of the file instead of the expected actual source file in the VScode editor.

ThatBrianDude commented 1 year ago

Experiencing the same issue. Making all the imports in the given file single-line fixes the issue. However this is not a viable solution as its incompatible with clean code principles.

Is there a working solution that supports multi-line imports?

AlanWarren commented 1 year ago

I'm also experiencing this issue. It seems we cannot debug JS files with vite at the moment, which is a bit hard to believe. I'm curious if maybe I've just missed something simple?

Chrome devtools skips over breakpoints in JS files. If I hit CMD+P to try and locate a JS file for debugging, devtools lists about 10 versions for each JS file, each with a different "t" query param.

IntelliJ IDE's just throw an error stating it cannot locate the file. It does appear to have the ability to manually map URL's to disk paths, but I haven't been able to get it working.

Is it still possible to monkey patch vite to generate sourcemaps for JS?

For the time being it seems renaming JS files to JSX works, but this isn't really desirable for pure JS code. i.e redux actions, etc.

Would appreciate any tips.

BenceSzalai commented 1 year ago

Steps 3-6.) here would still be needed to potentially fix this.

Alternatively you can check if the fix in vitejs/vite#9476 could be either monkey patched or merged on a fork on top of the current version of Vite, but that would most probably be not trivial, as that had its first birthday just these days, so the code it refers to most probably has changed a lot already since.

Another alternative can be to switch to TypeScript. I know, it would be strange to switch for this reason, but we did after all. Indeed not only because of this issue, but for many other reasons, but it was a push towards that direction.

sapphi-red commented 1 year ago

Vite 5.0.0-beta.1 includes #13514 that might fix this. Would anyone try it with IntelliJ IDE's?

BenceSzalai commented 1 year ago

I've did a quick check and it seems like to be much better than before with few caveats:

There's still a problem with proper mapping to the exact source file for rewritten .js files. What does it mean, is that the IDE cannot properly identify which source file corresponds to the one seen by the browser. What is better that at least with the present source map the IDE can apply some heuristics to try to identify the related source file, which may or may not work well.

Let's say I have a file like: vite-root/src/foo/bar.js. The sourcemap embedded in the file will have the information "sources":["/src/foo/bar.js"].
But when we compare it with another file: vite-root/src/foo/baz.ts, the sourcemap in this file will have this: "sources":["baz.ts"].

Now I cannot argue which one is better, but what I can say for sure that both cannot be good, and also what I can say is that IntelliJ IDE is able to identify baz.ts correctly all the time, while it cannot reliably identify bar.js.

Surely enough, if a simple project when there's only one file that matches /src/foo/bar.js path segment, IntelliJ will - I assume applying some heuristics - deduce that it must be the right file. But if there's another file that is matching the path segment /src/foo/bar.js the IDE may choose that file for the debugging display. This can easily happen in a monorepo + workspaces setup when some packages are visible for the IDE under both the source path and both under node_modules/packagename/src/foo/bar.js. It is also possible to have multiple different folders opened in the ide with similar paths, e.g.: vite-root/src/foo/bar.js, some-other-folder/src/foo/bar.js or even vite-root/another-folder/src/foo/bar.js. In these cases the IDE will show the debugger with the first file it finds, which may very well be a completely wrong file.

This suggests that the "sources":["/src/foo/bar.js"] in the sourcemap is not exact. The IDE can find the file based on it when there's only one match, but may miss it if there are multiple. If we compare it with the TypeScript behaviour, where "sources":["baz.ts"] is in the sourcemap, we can see that is working properly all the time, no matter how many other places contain baz.ts file. Based on this I think the sources in the sourcemap should be relative to the path where the sourcemap is (or the file the sourcemap is embedded into). This would explain why a bare baz.ts works perfectly all the time, but /src/foo/bar.js creates ambiguity.

Another strange thing I've noticed is that the view the browser (both Chrome and Firefox) reconstructs for the original file contains the last line //# sourceMappingURL=data:application/json;base64,... while this is not the case for .ts files. This would not affect debugging I think, as this is an additional line at the end which noone could set breakpoints against anyway, but still it is a notable difference compared to "normal" (i.e. conventional) behaviour. I guess the reson for this is that the sourcemap of .ts files contain a sourcesContent section, so the browsers would use that, while the sourcemap for the .js files does not have this, only the mappings section, which allows the browser to reconstruct (albeit probably with additional computation) the parts of the original source corresponding to the modified file served by Vite, but it cannot reconstruct the last //# sourceMappingURL=... line into anything in the original source, so it is just left there as it is at the end. But apparently this difference is ignored by IntelliJ IDE, again I assume based on a heuristics that says ignore inlined sourcemaps when comparing the file seen by the editor and the file contents sent by the browser. At the end of the day having sourcesContent in these maps may be preferable if no other reasons present to omit that part - I know there were performance concerns about full-blown sourcemaps. But I think adding the sourcesContent would be a negligible cost compared to generating the mappings section, as the sourcesContent is basically just inlining of the source file already loaded by Vite.

vitejs / vite