vue-loader icon indicating copy to clipboard operation
vue-loader copied to clipboard

Source maps generation adds broken intermediate code

Open rjgotten opened this issue 5 years ago • 9 comments

Version

15.4.1

Reproduction link

https://github.com/NetMatch/vue-loader-testcase

Steps to reproduce

  1. Clone the linked repository
  2. Execute npm ci
  3. Execute npm run build
  4. Inspect the generated source maps in the dist folder

What is expected?

vue-loader generates source maps for the source Vue SFC file and the constituent template, style and script sections.

The appendExtension option is honored and the source map lists the constituents as Example.vue.html, Example.vue.css and Example.vue.js.

What is actually happening?

vue-loader generates source maps for intermediate stages of generated JS code rather than the original sources of the template, style and JS sections.

The appendExtension option is not honored. All generated files are generated under the Example.vue name and are disambiguated by adding part of their content hash as a suffixed query string.


Same issue as in #1163 where the root cause was claimed to be Babel and should have been resolved with latest versions of all build tools. However, the test case presented here uses the latest versions of Webpack and vue-loader and doesn't even use Babel at all. And still has this problem.

rjgotten avatar Sep 06 '18 14:09 rjgotten

Just adding my 2 cents,

appendExtension adds .ts for me but unfortunately still clutters the source map source folder in browser dev tool with multiple .vue intermediate stage files...

It would be nice if we can have the original Vue SFC file being displayed instead of the intermediate files...

I may understand why vue-loader decides to add intermediate files in the source map; probably for debugging the vue-loader output itself during internal development.

However, seeing that the source map usually only is needed for debugging the consumer source code in the browser dev tool, perhaps don't show the intermediate files? (End-users tend not to put breakpoints in html and css parts of the template)

TL;DR hide the intermediate files behind a flag or something.

Thanks for considering.

ryanelian avatar Oct 16 '18 06:10 ryanelian

@yyx990803

Veeery disappointing to not have heard any response whatsoever to this issue for close to 2 months now, after you originally explicitly asked in #1163 to open a new issue, should the problem persist.

Not asking for an immediate solution, but a simple "yeah, we're going to look into it" would be nice...

rjgotten avatar Oct 22 '18 19:10 rjgotten

So...

I've just attached a debugger and started stepping through the webpack compilation, source map generation etc.

vue-loader modifies the loaderContext.resourcePath to append changed extensions. This changed resource path is not present when Webpack calls ModuleFilenameHelpers.createFilename inside the SourceMapDevToolPlugin.

I.e. this whole thing probably starts with vue-loader using a false assumption that loaderContext.resourcePath is safely changeable.

rjgotten avatar Nov 23 '18 11:11 rjgotten

Also seeing the intermediate code output. This is breaking debugging entirely for Firefox and in weird ways in Chrome. Chrome debugging seems to find the right code if the .vue file is the first entry in the stack trace, but not if it is isn't (i.e., the .vue file code is calling code in another module that throws an exception).

chrisnicola avatar Jan 10 '19 15:01 chrisnicola

I.e. this whole thing probably starts with vue-loader using a false assumption that loaderContext.resourcePath is safely changeable.

I've been investigating this for the past 4 days or so, and it's actually a bit more complicated than that. I don't think loaderContext.resourcePath has anything to do with it.

So, what I've found is:

A single .vue file (with script and template) will generate roughly 4 + numStyles + (2 * numCustomTags) entries in the sources list in a source map file. So for a .vue file with one <style> and one <custom> tag, you'll get 7:

  1. The original .vue file, with all of the root tags in place. This will be something like src/MyComponent.vue. This is where the <script> tag's source map points to.
  2. One for the "intermediate representation". This is a generated file which imports each section of your .vue file (transforming each section into JavaScript), normalizes, then returns the final component. The path will be something like /code/your-project/src/MyComponent.vue (note the absolute path).
  3. The <template> tag, compiled to JavaScript. The path is something like like /code/your-project/src/MyComponent.vue?vue&type=template&id=12345&scoped=true
  4. A "pseudo-file" which simply imports your <script> tag using the correct loaders from your webpack config. The path is something like like /code/your-project/src/MyComponent.vue?vue&type=script&lang=js&.
  5. A "pseudo-file" which simply imports your <style> tag using the correct loaders from your webpack config. The path is something like /code/your-project/MyComponent.vue?vue&type=style&index=0&id=12345&scoped=true&lang=css&
  6. A "pseudo-file" which simply imports your <custom> tag using your custom tag loader configured in webpack. The path is something like /code/your-project/src/MyComponent.vue?vue&type=custom&index=0&blockType=tagName
  7. The resulting JavaScript of your custom tag loader. The path for this is identical to the previous one (however the identifier is different because it uses a different loader).

I don't know the best fix for this, however my current assumption is that if 2-7 included source maps when returning code from the loader, with a source path equivalent to number 1, then there would only be one entry in the sources file.

So for starters, the pitcher.js file (pitcher loader), would need to include source maps for its "redirect imports". For example, for 5, it does this:

return `import mod from ${request}; export default mod; export * from ${request}`

Instead, it should do something like this:

const sourceMap = generateSourceMapToOriginalFile(etc);
this.callback(null, `import mod from ${request}; export default mod; export * from ${request}`, sourceMap)

Same for the templates and custom blocks in that file. That gets rid of 4, 5, and 6.

However, I'm guessing the pitcher might go away in the next major version because Webpack added the matchResource feature (I'm guessing specifically for vue-loader?), which might eliminate the need for the pitcher loader altogether.

The <template> tag needs to generate source maps on its own. I believe that's a separate issue in this repo, and it's planned for 3.0. This gets rid of 3.

Custom blocks are up to your custom block code. That would get rid of 7.

Finally, that just leaves the "intermediate representation". The index.js file in vue-loader is mainly responsible for generating this file. It would need to generate source maps along the way, and then at the end, instead of

return code

It would need to do,

loaderContext.callback(null, code, sourceMap)

Which gets rid of 2. Then all source maps would point to the same file as 1 (the original), and in theory everything would work.

Let me know if I'm incorrect in any of this, I'm new to webpack development!

abrenneke avatar Mar 26 '19 08:03 abrenneke

Which gets rid of 2. Then all source maps would point to the same file as 1 (the original), and in theory everything would work.

That still ends up with a pool of source maps all pointing back at 1, but sharing the same name. Each would clobber whatever was registered under that name previously. And the arbitrary last entrant would 'win'.

Webpack concatenates a query string with the first few symbols of the hash fingerprint of a file when it detects a source map name collision, exactly to avoid this problem.

Vue loader fails to correctly propagate faked split names (e.g. Component.vue -> Component.vue.css ; Component.vue.html ; Component.vue.js) which causes all the source maps to be published under the same origin name Component.vue. This kicks in Webpack's collision prevention and leads to the ugly 'duplicated' sourec maps.

Vue loader should:

  1. Fix split-part source maps to have a proper name. This involves changing the mechanism that currently relies on loaderContext.resourcePath, because this is NOT able to be safely modified.
  2. Offer a working option to turn these split source maps off altogether, which means not emitting them; i.e. returning an empty or null sourcemap for them.
  3. Remove the intermediate representation altogether. It's literally a useless internal implementation detail. At best it should be behind a verbose debugging switch that is off by default.

rjgotten avatar Mar 26 '19 09:03 rjgotten

Remove the intermediate representation altogether. It's literally a useless internal implementation detail. At best it should be behind a verbose debugging switch that is off by default.

The problem is that when a module, any module, is resolved by webpack, it generates a source map. The way vue-loader works is by resolving each .vue file, as a module, one time per <block> (plus more because of pitching). Each .vue file emits 5+ webpack modules.

Emitting an empty source map would probably work like you said.

The relevant createSource function in NormalModule.js, which, from what I can tell, is always run for every single module.

Note this line at the end, which is the default for a module:

return new OriginalSource(source, identifier);

identifier is the full string for a module, including every loader used to process it, as well as query params at the end. Since vue-loader generates these dynamically, each stage has its own identifier. No collision detection kicks in because they have separate identifiers.

If all of the intermediate modules have a source map to the original file, I think that the source map generation is smart enough to merge them all. Maybe I'm wrong on that one, I haven't tried.

Edit:

There is collision detection with devtoolModuleFilenameTemplate and devtoolFallbackModuleFilenameTemplate. The job of those two functions is to rewrite the long identifier I talked about above into a friendly file name to show in the source maps. It prevents any two entries in the sources array from having the same name. It's separate from actually merging source maps together, I still believe that earlier on in the process, if two source maps pointed to the same source, that there would only be one entry in the sources array in the first place.

abrenneke avatar Mar 26 '19 09:03 abrenneke

What is the status with this issue? Will it ever be addressed? What can I do to help move it along?

It's very frustrating that Vue template errors can not be mapped to source. When our react native apps have errors in production the Sentry error shows exactly the line in source where the error occurred but for Vue we just get garbage.

I think this is the same issue as https://github.com/vuejs/vue-cli/issues/2978.

ncjones avatar Mar 06 '21 22:03 ncjones

Yeah this essentially renders vue unusable. How do you expect people to use an application in production if they can't debug it? I understand there might be fixes in the cases, but they're not guaranteed to work and I would expect this to work out of the box