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

Webpack is producing invalid sourcemaps? #8302

Closed
adaniliuk opened this issue Oct 29, 2018 · 44 comments

Comments

@adaniliuk
Copy link

commented Oct 29, 2018

tl;dr

  • This issue was open before and marked as fixed #7616, but unfortunately, it's still reproducible with the latest version of webpack
  • According to https://sourcemaps.io/ and sourcemap-validator, the sourcemaps produced by webpack are invalid
  • using devtool: "source-map", a barebones webpack config, and webpack@4.23.1
  • sample repro repo available at https://github.com/adaniliuk/webpack-sourcemap-testing
    (this might be an issue with sourcemap-validator, but the error that it's throwing (in the "current behavior" section below) seems pretty legit)

Bug report

What is the current behavior?

Running webpack with the following config:

const webpack = require('webpack');
const path = require('path');

module.exports = {
    entry: './src/index.js',
    output: {
        path: path.resolve(__dirname, 'dist'),
        filename: 'bundle.js'
    },
    devtool: 'source-map'
};
  • produces dist/bundle.js and dist/bundle.js.map.
  • running those files through sourcemap-validator (or https://sourcemaps.io/, and consequently Sentry) produces the following error:
/webpack-sourcemap-testing/node_modules/sourcemap-validator/index.js:146
          throw new Error(errMsg);
          ^
Error: Warning: mismatched names
Expected: installedModules || 'installedModules' || 'installedModules' || "installedModules" || "installedModules"
Got:  	var installedM ||  	var installedMod ||  	var installedMod ||  	var installedMod ||  	var installedMod
Original Line:  	var installedModules = {};
Mapping: 1:17→2:0 "installedModules" in webpack:///webpack/bootstrap
    at /webpack-sourcemap-testing/node_modules/sourcemap-validator/index.js:146:17
    at Array.forEach (<anonymous>)
    at SourceMapConsumer_eachMapping [as eachMapping] (/webpack-sourcemap-testing/node_modules/sourcemap-validator/node_modules/source-map/lib/source-map/source-map-consumer.js:570:10)
    at validate (/webpack-sourcemap-testing/node_modules/sourcemap-validator/index.js:83:12)
    at Object.<anonymous> (/webpack-sourcemap-testing/test-sourcemaps.js:6:1)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)

If the current behavior is a bug, please provide the steps to reproduce.

Repro repo here: https://github.com/adaniliuk/webpack-sourcemap-testing

Repro instructions for that repo are on the readme.

If starting from scratch, use the following webpack config:

const webpack = require('webpack');
const path = require('path');

module.exports = {
    entry: './src/index.js',
    output: {
        path: path.resolve(__dirname, 'dist'),
        filename: 'bundle.js'
    },
    devtool: 'source-map'
};

and use sourcemap-validator to test dist/bundle.js for validity.

What is the expected behavior?

  • webpack produces valid source maps
  • so sourcemap-validator reads the sourcemap as valid (e.g., not throw an error).

It looks like it might have something to do with some misaligned whitespace, but I'm really not sure.

Other relevant information:

webpack version: 4.23.1
Node.js version: 8.11.3
Operating System: macOS 10.14 (18A391)
Additional tools: sourcemap-validator@1.1.0, yarn@1.9.2

@adaniliuk

This comment has been minimized.

Copy link
Author

commented Oct 29, 2018

@ktalebian

This comment has been minimized.

Copy link

commented Dec 12, 2018

I'm observing the same issue as well.

@kenhoff

This comment has been minimized.

Copy link

commented Dec 27, 2018

Reposting over here - sorry about the delay on this. I ran through my sample repo (https://github.com/kenhoff/webpack-sourcemap-testing) and updated to the latest versions of everything. Appears to still be an issue.

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Dec 27, 2018

@kenhoff thanks, need investigate who break source map 😕

@adaniliuk

This comment has been minimized.

Copy link
Author

commented Dec 28, 2018

Just for the record. We have found out that in our case issue was in the optimizations CDN did to JS bundles before sending them to the clients. So source code didn't match the map.

Though the above issue still exists and source map validation fails.

@jlchereau

This comment has been minimized.

Copy link

commented Jan 9, 2019

Invalid sourcemaps with v4.28.1

@justingrant

This comment has been minimized.

Copy link

commented Jan 29, 2019

Is this issue the root cause of facebook/create-react-app#6296 ? That issue is caused by whitespace differences (newlines and indentation only) between sourcemap source code and on-disk source code.

@theshem

This comment has been minimized.

Copy link

commented Apr 2, 2019

Any updates on this?

Still get invalid sourcemaps with v4.29.6

@christophpre

This comment has been minimized.

Copy link

commented Apr 23, 2019

Invalid source-maps with 4.29.6 and 4.30.0.

devtool: 'source-map'
babel-loader
babel/preset-env

Although it doesn't seem to have anything to do with babel, since I get invalid source maps (wrong line number) even when removing babel completely.

Edit:
For now I'm using eval-source-map, since this seems to work.

@september28

This comment has been minimized.

Copy link

commented May 3, 2019

Also an issue for me!
I was on node v10.15.3 with npm v6.4.1 and I had the issue.
Just updated to npm v6.9.0 and the issue is still present.
Webpack version is 4.30.0
Running a sourcemap through a validator produces:

Error: Warning: mismatched names
Expected: installedModules || 'installedModules' || 'installedModules' || "installedModules" || "installedModules"
Got:  	var installedM ||  	var installedMod ||  	var installedMod ||  	var installedMod ||  	var installedMod
Original Line:  	var installedModules = {};
Mapping: 1:17→2:0 "installedModules" in webpack:///webpack/bootstrap
...
@september28

This comment has been minimized.

Copy link

commented May 10, 2019

This issue is actually preventing me from launching my Firefox addon since I need to send sourcemaps to Mozilla for review. I have read the thread about the Facebook React apps as mentioned by @justingrant and tried the suggestions there with no luck.

In desperation, I searched for all instances of "var installedModules" inside the node_modules directory, and deleted any whitespace character (starting with spaces and tabs, then removing the /*****/). I tested by building and validating sourcemap after each file change. I found NO change in the result despite having changed every file I came across!

I have since concluded that this issue centres around the file MainTemplate.js (and associated files/classes/methods) (node_modules/webpack/lib/MainTemplate.js) - there is something that it is outputting that the sourcemap generator cannot deal with correctly. Changing line 427 does have an effect on the issue:
Template.prefix(buf, " \t") + "\n",
So for example, if we change the prefix to just an empty string (Template.prefix(buf, "") + "\n",), the resulting validation error changes by the same 2 chars that we have now deleted...

Error: Warning: mismatched names
Expected: installedModules || 'installedModules' || 'installedModules' || "installedModules" || "installedModules"
Got: var installedMod || var installedModul || var installedModul || var installedModul || var installedModul
Original Line: var installedModules = {};
Mapping: 1:17→2:0 "installedModules" in webpack:///webpack/bootstrap

(Notice the line "Got: var installedMod...")

Still doesn't solve the issue, but gets a little closer. I suspect that the real issue here is not in this file, rather how the output is being parsed by the source map generator to produce the mappings....

Apologies a rather layman approach, but I am no expert in this area and am desperate to find a solution - I'm suprised this is not crippling others?

@justingrant

This comment has been minimized.

Copy link

commented May 10, 2019

FYI, the problems I was running into with React turned out to be fixed by changing Babel configuration. If you configure Babel with sourceMaps: false and webpack with devtool: 'cheap-module-source-map' then webpack still emits sourcemaps for the bundled code, but those sourcemaps are invalid. Specifically, the sourcesContent and mappings are internally consistent, but neither of them match the original source code on disk. Specifically, the sourcesContent differs from the original source in line-breaks and comments.

If you change Babel config to use sourceMaps: true, inputSourceMap: true then sourcesContent and mappings will match the on-disk source files.

facebook/create-react-app#7022 has more details about the problem and the solution. The TL;DR is that if Babel is used, then sourceMaps: true, inputSourceMap: true is required in order to emit valid sourcemaps.

Unfortunately, I can't comment about the variable name mapping issues that @september28 is running into above, because create-react-app sourcemaps don't even include names mappings at all. (Is there a webpack and/or babel config option to enable name mapping in sourcemaps?)

@september28

This comment has been minimized.

Copy link

commented May 24, 2019

OK, I have done some more investigation which I will share below. Would be really grateful if someone from the Webpack team could take a look at this issue since it would seem pretty fundamental to most projects?

OK, so my thinking is that this issue has to do with the Terser plugin source mapping that is passed from the webpack(?) loader (or maybe even the SourceMapDevToolPlugin) to Terser's minify method, which is the default minifier shipped with Webpack. I have come to thinking this because if the code is not minified during build (set optimization -> minimize to false), then the resulting source map(s) validate correctly (obviously having a sourcemap for non-minifed/non-mangled code is not necessary, but it seems to help isolate where the issue is).

So I began by "overriding" the minify function of the Terser Plugin. Here's the relevant section of my webpack.config.js:

optimization: {
        minimize: true,
        minimizer: [
            new TerserPlugin({
                cache: false,
                parallel: true,
                sourceMap: true, // Must be set to true if using source-maps in production
                minify: (file, sourceMap) => {
                    if(file["background.js"]) {
                        console.log("file: ", file);
                        console.log("sourceMap: ", sourceMap);
                    }
                    return false;
                },
                terserOptions: {
                    // https://github.com/webpack-contrib/terser-webpack-plugin#terseroptions
                    ecma: undefined,
                    warnings: false,
                    parse: {},
                    compress: {},
                    mangle: true, // Note `mangle.properties` is `false` by default.
                    module: false,
                    output: null,
                    toplevel: false,
                    nameCache: null,
                    ie8: false,
                    keep_classnames: undefined,
                    keep_fnames: false,
                    safari10: false,
                }
      }),
    ],
    },

As you can see, my minify function is just console.logging the two parameters that have been passed to it. My first observation is that the "file" code is different from the sourceMap's "sourcesContent" code - perhaps the first indicator that something is not quite right?

(Abbreviated) file param:

{ 'background.js':
   '/******/ (function(modules) { // webpackBootstrap\n/******/ \t// The module cache\n/******/ \tvar installedModules = {};\n/******/\n/******/ \t// The require function\n

(Abbreviated) sourceMap param:

file: 'x',
  sourcesContent:
   [ ' \t// The module cache\n \tvar installedModules = {};\n\n

Now, even though this doesn't quite make sense to me (that file code should be different to the sourceMap sourcesContent), The minifier is going to completely change this file code anyway, so it will have to create a new source map for the code. I took a look at how the TerserPlugin does this.

node_modules/terser-webpack-plugin/minify.js calls terser's minify function with the file content, and a terserOptions object. inside that object, the sourceMap has been injected:

terserOptions.sourceMap = {
      content: inputSourceMap
    };

node_modules/terser/dist/bundle.js defines the minify function. Around line 20671 seems to be the relevant source map code:

if (options.sourceMap) {
                  if (typeof options.sourceMap.content == "string") {
                      options.sourceMap.content = JSON.parse(options.sourceMap.content);
                  }
                  options.output.source_map = SourceMap({
                      file: options.sourceMap.filename,
                      orig: options.sourceMap.content,
                      root: options.sourceMap.root
                  });
                  if (options.sourceMap.includeSources) {
                      if (files instanceof AST_Toplevel) {
                          throw new Error("original source content unavailable");
                      } else for (var name in files) if (HOP(files, name)) {
                          options.output.source_map.get().setSourceContent(name, files[name]);
                      }
                  }
              }

so, the original sourceMap (from, presumably, the webpack loader or whatever part of webpack that "feeds" the minimize function) gets converted into a JSON object, and then a SourceMap is instantiated using the original source map content.

the SourceMap class is defined earlier in the same bundle.js file, and commented as "a small wrapper around fitzgen's source-map library". It's basically a wrapper around MOZ_SourceMap. It looks like there is a node_modules sub-heirarchy inside the terser package, so I think the SourceMapGenerator class is defined in node_modules/terser/node_modules/source-map/lib/source-map-generator.js.

So up to this point we know that the sourceMap mappings are going to be incorrect since the code has been minified/mangled. There is an OutputStream class that is called in the minify function which handles the "rewrite" of the sourcemap mappings.

var do_add_mapping = mappings ? function() {
          mappings.forEach(function(mapping) {
              try {
                  options.source_map.add(
                      mapping.token.file,
                      mapping.line, mapping.col,
                      mapping.token.line, mapping.token.col,
                      !mapping.name && mapping.token.type == "name" ? mapping.token.value : mapping.name
                  );
              } catch(ex) {
                  mapping.token.file != null && AST_Node.warn("Couldn't figure out mapping for {file}:{line},{col} → {cline},{ccol} [{name}]", {
                      file: mapping.token.file,
                      line: mapping.token.line,
                      col: mapping.token.col,
                      cline: mapping.line,
                      ccol: mapping.col,
                      name: mapping.name || ""
                  });
              }
          });
          mappings = [];
      } : noop;

So this do_add_mapping method calls the "add" method from the SourceMap class (wrapper):

function add(source, gen_line, gen_col, orig_line, orig_col, name) {
          if (orig_map) {
              var info = orig_map.originalPositionFor({
                  line: orig_line,
                  column: orig_col
              });
              if (info.source === null) {
                  return;
              }
              source = info.source;
              orig_line = info.line;
              orig_col = info.column;
              name = info.name || name;
          }

          generator.addMapping({
              generated : { line: gen_line + options.dest_line_diff, column: gen_col },
              original  : { line: orig_line + options.orig_line_diff, column: orig_col },
              source    : source,
              name      : name
          });
      }

So I think the important thing here is the conditional if(orig_map). My hypothesis is that the orig_map (which has been essentially passed through to terser from webpack) has incorrect mappings (the "non-minified source" line/col mapping looks incorrect given the error message I get when trying to validate - the mapping seems to always map a name to a correct looking line but column 0, when the column should be somewhere around 14 to 17 i think).

interestingly the orig_line and orig_col passed to this add method look different (indeed more correct) - I think they have been generated when we created the SourceMap (see above code snippet): options.output.source_map.get().setSourceContent(name, files[name]);

Looking in the source-map-generator.js file, the addMapping function:

/**
 * Add a single mapping from original source line and column to the generated
 * source's line and column for this source map being created. The mapping
 * object should have the following properties:
 *
 *   - generated: An object with the generated line and column positions.
 *   - original: An object with the original line and column positions.
 *   - source: The original source file (relative to the sourceRoot).
 *   - name: An optional original token name for this mapping.
 */
SourceMapGenerator.prototype.addMapping =
  function SourceMapGenerator_addMapping(aArgs) {
    var generated = util.getArg(aArgs, 'generated');
    var original = util.getArg(aArgs, 'original', null);
    var source = util.getArg(aArgs, 'source', null);
    var name = util.getArg(aArgs, 'name', null);

    if (!this._skipValidation) {
      this._validateMapping(generated, original, source, name);
    }

    if (source != null) {
      source = String(source);
      if (!this._sources.has(source)) {
        this._sources.add(source);
      }
    }

    if (name != null) {
      name = String(name);
      if (!this._names.has(name)) {
        this._names.add(name);
      }
    }

    this._mappings.add({
      generatedLine: generated.line,
      generatedColumn: generated.column,
      originalLine: original != null && original.line,
      originalColumn: original != null && original.column,
      source: source,
      name: name
    });
  };

So this function does not seem to remove mappings? So Is it the case that we are just adding to the eroneous mappings that we know already exist in the source map?

So my investigation is now going to focus on how the sourceMap is generated that is passed to the terser minify function - I believe this is generated during "compilation" either by a loader, or perhaps by the SourceMapDevToolPlugin (it is certainly called before the minify function).

Any help greatly appreciated!!

As a side note, the version of the source map library that terser seems to be using is fairly old now (0.5.6) - this is probably because, after that point, there were some fairly major changes made to Promiseify the code etc - ChangeLog

@connorjclark

This comment has been minimized.

Copy link
Contributor

commented May 24, 2019

@september28 - thanks for sharing. Haven't fully dived into your findings, but I wanted to share some work I did on source map handling in Terser: terser-js/terser#298. Maybe it doesn't apply - haven't looked closely.

With your setup, could you try using this branch w/ the "auto" feature? Or, if you could upload a repo of your setup I could (it's been awhile since I've setup any webpack :) )

@september28

This comment has been minimized.

Copy link

commented Jun 3, 2019

Hi @hoten thanks for the reply - however I don't think your work on terser-js/terser#298 is relevant to the issue. Correct me if I'm wrong, but your mods seem to address an issue with finding sourcemaps from the #SourceMappingURL comment, or searching for a file.map.js if the special comment doesn't exist?

@september28

This comment has been minimized.

Copy link

commented Jun 3, 2019

I just tried the same scripts installed on a Windows 10 machine to rule out the system architecture. Still no joy. Is anyone from Webpack tracking this issue? It is pretty fundamental to JS projects in Webpack...

@september28

This comment has been minimized.

Copy link

commented Jun 5, 2019

OK so I think this I have isolated this down to the way that webpack generates sourcemaps for "required" files when bundling. I changed the sourcemap-validator code (node_modules/sourcemap-validator/index.js) around line 146:

if(!success) {
        mapRef = template('<%=generatedLine%>:<%=generatedColumn%>'
                          + fancyArrow
                          + '<%=originalLine%>:<%=originalColumn%> "<%=name%>" in <%=source%>')(mapping);
        errMsg = template(''
          + 'Warning: mismatched names\n'
          + 'Expected: <%=expected%>\n'
          + 'Got: <%=actual%>\n'
          + 'Original Line: <%=original%>\n'
          + 'Mapping: <%=mapRef%>'
          , {
            expected: expected.join(' || ')
          , actual: actuals.join(' || ')
          , original: originalLine
          , mapRef: mapRef
          });

          //throw new Error(errMsg);
          if(mapping.source.substring(0,7) !== "webpack") console.log(errMsg);
      }
    }

Basically I stopped the code from throwing an exception, opting instead for a console.log message. Originally I just did a console.log(errMsg) - This throws up hundreds of mapping errors. One thing I noticed was that they seem to be all referring to files with the webpack:// prefix e.g.:

Warning: mismatched names
Expected: hub || 'hub' || 'hub' || "hub" || "hub"
Got:     ||     c ||     c ||     c ||     c
Original Line:     carrier.__SENTRY__.hub = hub;
Mapping: 1:61028→381:0 "hub" in webpack:///node_modules/@sentry/hub/esm/hub.js

There were so many that my terminal window ran out of buffer so I could not analyse them all. I then added the conditional in the above code to see if there were any errors in the actual js file that I had written, since this would have been transpiled/minified by webpack (probably at a separate stage) and then concatenated into the final minified file (along with the other "required"-in modules).

There are no validation errors shown for anything other than the webpack:// "files", which leads me to add more weight towards my theory that there is something going wrong with webpack when the required modules are being transpiled/minified and sourcemaps are being written.

@september28

This comment has been minimized.

Copy link

commented Jun 6, 2019

After some more digging, it would seem that the "original" column mappings are not happening correctly. I have liberally sprinkled console.log statements in: "node_modules/webpack-sources/node_modules/source-map/lib/source-node.js", specifically in the SourceNode.prototype.toStringWithSourceMap = function SourceNode_toStringWithSourceMap(aArgs) { function and found that whilst the "generated" (minified) mappings look correct (both line and column), the "original" line looks correct however the "original" column is nearly always 0, which would explain the error that I am seeing (it should usually be > 0 because the line will start something like "var installedModules = {}" therefore "installedModules" starts at column 4, not 0.
"original" mappings are SourceNodes objects I believe, mostly created in webpack-sources/lib/OriginalSource.js I believe where there doesn't seem to be any code to suggest that column mappings to variable names have been considered...
Not sure where to go from here... HELP PLEASE SOMEONE!

@september28

This comment has been minimized.

Copy link

commented Jun 6, 2019

cc @edmorley because he was involved in #7616 , sorry #41

@connorjclark

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2019

I believe column mapping is configurable in source map generation (maps are smaller if you don't embed column info). Maybe there's a misconfiguration in some tool?

@september28 I've set aside time this weekend to dive deeper into this topic. It's bugged me for a long time too.

@september28

This comment has been minimized.

Copy link

commented Jun 6, 2019

Hi @hoten , yep column mapping is configurable. In my examples options.columns passed to node function in OriginalSource.js is always undefined, which is fine since I guess the sourcemapping assumes that you want column mappings unless you set specifically to false.
Yay about the weekend :-) let me know if you want me to do/test anything...

@connorjclark

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

Okay, I've got some news. Let me describe my debugging approach first.

My config:

const path = require('path');

module.exports = {
    entry: './index.js',
    output: {
        path: path.resolve(__dirname, 'dist'),
        filename: 'bundle.js'
    },
    devtool: 'source-map'
};

index.js is just console.log(123);.

I use ndb to step through the code. I also used surge.sh to quickly test against the source map validator (I know there's a npm package too, but this was faster). Some modifications I made (I wish there were a tool to diff a dirty node_modules!):

./node_modules/terser/package.json

Change main to dist/bundle.js, and delete ./node_modules/terser/dist/bundle.js.map. The first part will load the non-minified module (so its readable in the debugger) and the second part will prevent ndb from transforming stack frames to the original terser source files (because I want to modify dist/bundle.js).

./node_modules/terser-webpack-plugin/dist/minify.js

Add console.log(inputSourceMap.sourcesContent[0]); ~L161. This gives a nice look at the input source map provided by Webpack to Terser. Output:

Click to expand
 	// The module cache
 	var installedModules = {};

 	// The require function
 	function __webpack_require__(moduleId) {

 		// Check if module is in cache
 		if(installedModules[moduleId]) {
 			return installedModules[moduleId].exports;
 		}
 		// Create a new module (and put it into the cache)
 		var module = installedModules[moduleId] = {
 			i: moduleId,
 			l: false,
 			exports: {}
 		};

 		// Execute the module function
 		modules[moduleId].call(module.exports, module, module.exports, __webpack_require__);

 		// Flag the module as loaded
 		module.l = true;

 		// Return the exports of the module
 		return module.exports;
 	}


 	// expose the modules object (__webpack_modules__)
 	__webpack_require__.m = modules;

 	// expose the module cache
 	__webpack_require__.c = installedModules;

 	// define getter function for harmony exports
 	__webpack_require__.d = function(exports, name, getter) {
 		if(!__webpack_require__.o(exports, name)) {
 			Object.defineProperty(exports, name, { enumerable: true, get: getter });
 		}
 	};

 	// define __esModule on exports
 	__webpack_require__.r = function(exports) {
 		if(typeof Symbol !== 'undefined' && Symbol.toStringTag) {
 			Object.defineProperty(exports, Symbol.toStringTag, { value: 'Module' });
 		}
 		Object.defineProperty(exports, '__esModule', { value: true });
 	};

 	// create a fake namespace object
 	// mode & 1: value is a module id, require it
 	// mode & 2: merge all properties of value into the ns
 	// mode & 4: return value when already ns object
 	// mode & 8|1: behave like require
 	__webpack_require__.t = function(value, mode) {
 		if(mode & 1) value = __webpack_require__(value);
 		if(mode & 8) return value;
 		if((mode & 4) && typeof value === 'object' && value && value.__esModule) return value;
 		var ns = Object.create(null);
 		__webpack_require__.r(ns);
 		Object.defineProperty(ns, 'default', { enumerable: true, value: value });
 		if(mode & 2 && typeof value != 'string') for(var key in value) __webpack_require__.d(ns, key, function(key) { return value[key]; }.bind(null, key));
 		return ns;
 	};

 	// getDefaultExport function for compatibility with non-harmony modules
 	__webpack_require__.n = function(module) {
 		var getter = module && module.__esModule ?
 			function getDefault() { return module['default']; } :
 			function getModuleExports() { return module; };
 		__webpack_require__.d(getter, 'a', getter);
 		return getter;
 	};

 	// Object.prototype.hasOwnProperty.call
 	__webpack_require__.o = function(object, property) { return Object.prototype.hasOwnProperty.call(object, property); };

 	// __webpack_public_path__
 	__webpack_require__.p = "";


 	// Load entry module and return exports
 	return __webpack_require__(__webpack_require__.s = 0);

./node_modules/terser/dist/bundle.js

I set a conditional breakpoint in do_add_mapping:

if (mapping.token.value === 'installedModules') debugger;

The token col is 14, which is unexpected because input source map printed above shows less indentation for this line - it should be something like 7. (I'm unclear on 0/1 index or if it includes the column before or of the first character of the token... so.... I expect 7-ish).

I want to see how this token / AST is made, so I step up a bit and see where Terser parses it. I added a debugger statement around L20578, to inspect the AST and confirm nothing is modifying it between here and do_add_mapping. I inspect toplevel and find the node for installedModules.

toplevel.body[0].body.expression.body[0].definitions[0].name === the installedModules node
toplevel.body[0].body.expression.body[0].definitions[0].name.start.col === 14

Also 14, which suggests I should stop looking inside Terser for the issue. I notice that the file input has a bunch of /******/ prefixes on each line.

Click to expand
"/******/ (function(modules) { // webpackBootstrap
/******/// The module cache
/******/var installedModules = {};
/******/
/******/// The require function
/******/function __webpack_require__(moduleId) {
/******/
/******/	// Check if module is in cache
/******/	if(installedModules[moduleId]) {
/******/		return installedModules[moduleId].exports;
/******/	}
/******/	// Create a new module (and put it into the cache)
/******/	var module = installedModules[moduleId] = {
/******/		i: moduleId,
/******/		l: false,
/******/		exports: {}
/******/	};
/******/
/******/	// Execute the module function
/******/	modules[moduleId].call(module.exports, module, module.exports, __webpack_require__);
/******/
/******/	// Flag the module as loaded
/******/	module.l = true;
/******/
/******/	// Return the exports of the module
/******/	return module.exports;
/******/}
/******/
/******/
/******/// expose the modules object (__webpack_modules__)
/******/__webpack_require__.m = modules;
/******/
/******/// expose the module cache
/******/__webpack_require__.c = installedModules;
/******/
/******/// define getter function for harmony exports
/******/__webpack_require__.d = function(exports, name, getter) {
/******/	if(!__webpack_require__.o(exports, name)) {
/******/		Object.defineProperty(exports, name, { enumerable: true, get: getter });
/******/	}
/******/};
/******/
/******/// define __esModule on exports
/******/__webpack_require__.r = function(exports) {
/******/	if(typeof Symbol !== 'undefined' && Symbol.toStringTag) {
/******/		Object.defineProperty(exports, Symbol.toStringTag, { value: 'Module' });
/******/	}
/******/	Object.defineProperty(exports, '__esModule', { value: true });
/******/};
/******/
/******/// create a fake namespace object
/******/// mode & 1: value is a module id, require it
/******/// mode & 2: merge all properties of value into the ns
/******/// mode & 4: return value when already ns object
/******/// mode & 8|1: behave like require
/******/__webpack_require__.t = function(value, mode) {
/******/	if(mode & 1) value = __webpack_require__(value);
/******/	if(mode & 8) return value;
/******/	if((mode & 4) && typeof value === 'object' && value && value.__esModule) return value;
/******/	var ns = Object.create(null);
/******/	__webpack_require__.r(ns);
/******/	Object.defineProperty(ns, 'default', { enumerable: true, value: value });
/******/	if(mode & 2 && typeof value != 'string') for(var key in value) __webpack_require__.d(ns, key, function(key) { return value[key]; }.bind(null, key));
/******/	return ns;
/******/};
/******/
/******/// getDefaultExport function for compatibility with non-harmony modules
/******/__webpack_require__.n = function(module) {
/******/	var getter = module && module.__esModule ?
/******/		function getDefault() { return module['default']; } :
/******/		function getModuleExports() { return module; };
/******/	__webpack_require__.d(getter, 'a', getter);
/******/	return getter;
/******/};
/******/
/******/// Object.prototype.hasOwnProperty.call
/******/__webpack_require__.o = function(object, property) { return Object.prototype.hasOwnProperty.call(object, property); };
/******/
/******/// __webpack_public_path__
/******/__webpack_require__.p = "";
/******/
/******/
/******/// Load entry module and return exports
/******/return __webpack_require__(__webpack_require__.s = 0);
/******/ })
/************************************************************************/
/******/ ([
/* 0 */
/***/ (function(module, exports) {

console.log(123);


/***/ })
/******/ ]);

A search for /******/ brings us back to MainTemplate.js.

Consider this code:

image

Perhaps the issue lies with how ConcatSource / PrefixSource combines maps?

Next, I remove all the extraneous prefixes. If the original code is just:

var installedModules = {};

then I'd expect the original column to be 4 (or 5?).

I remove the PrefixSource and just add bootstrapSource directly. The original column is now 6. The error from source-map-validator is unchanged (In webpack:///webpack/bootstrap: Expected installedModules but got var installedM at L2:0).

I remove the \t on ~L443 of MainTemplate.js and now the original column is 4. This does change the error from source-map-validator, but it doesn't seem meaningful (In webpack:///webpack/bootstrap: Expected installedModules but got var installedMod at L2:0);

I start trashing some stuff. I minimize the MainTemplate render to just this, while maintaining the error:

const source = new ConcatSource();
// so webpack doesn't remove the dead code
source.add("window.blah = ");
source.add(
	this.hooks.modules.call(
		new RawSource(""),
		chunk,
		hash,
		moduleTemplate,
		dependencyTemplates
	)
);
return source;
In webpack:///index.js: Expected log but got con at L1:0

I print out the map here (console.log(source.map());) and see that the mappings are ;;;;AAAA - but the end result mappings are 2BAAAA,QAAAC,IAAA. There are no validator errors when using ;;;;AAAA. Maybe Webpack makes some more modifications before sending off to minify? I go back to ./node_modules/terser-webpack-plugin/dist/minify.js to check out the input source map given to Terser - it's still ;;;;AAAA. The map Terser returns is 2BAAAA,QAAAC,IAAA.

Remember when I said I stopped looking for the issue in Terser? Well, I guess that's busted now.

I'll take a break at this point :)

@september28

This comment has been minimized.

Copy link

commented Jun 11, 2019

@hoten Great progress... Seems like you have found broadly the same as me so far.

One comment I have about whether the issue lies with Webpack or Terser/Terser-webpack-plugin is that I have observed the following:
The inputSourceMap provided to terser-webpack-plugin's minify function seems to look correct (and may even validate correctly), however if you take a look at the entire sourcemap object, there are some things to note:

  • the names property of the inputSourceMap seems to only include names from the "user written" file (in my case background.js, in @hoten's case index.js, not any "ast" (e.g. installedModules is not there in that names array)
  • Nevertheless, the sources and sourcesContent properties list respectively all of the "files" and "files source code" (including webpack required files (ASTs)
  • the input property provided to terser-webpack-plugin's minify function is different from the sourcesContent in the inputSourceMap. Now, we would expect this to an extent, because the input will be a non-minified concatenation of all of the sources, whereas the inputSourceMap will have a non-minified array of the files separately (non-concatenated). HOWEVER, the installedModules-relevant code is different in the fact that in the inputSourceMap.sourcesContent array, the relevant source has had the /*****/ removed (but not whitespace chars), whereas the code in the input variable has these comment blocks retained.

Still not sure how much relevance this has, but thought I would share.

@connorjclark

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

It's 100F+ in the SF bay area, I have no A/C, and stumbling around this issue some more seems preferable to attempting to sleep :)

I print out the map here (console.log(source.map());) and see that the mappings are ;;;;AAAA - but the end result mappings are 2BAAAA,QAAAC,IAAA. There are no validator errors when using ;;;;AAAA. Maybe Webpack makes some more modifications before sending off to minify? I go back to ./node_modules/terser-webpack-plugin/dist/minify.js to check out the input source map given to Terser - it's still ;;;;AAAA. The map Terser returns is 2BAAAA,QAAAC,IAAA

OK, this part was nonsense. I didn't get validation errors using ;;;;AAAA because the source map didn't make much sense. Using https://sokra.github.io/source-map-visualization/ made that clearer. The ;;;;;AAAA is for this:

image

Which looks totally fine visualized:

image

Visualizing the output of my console.log(123)-only MainTemplate.js:

window.blah=[function(o,n){console.log(123)}];
//# sourceMappingURL=bundle.js.map
{"version":3,"sources":["webpack:///./index.js"],"names":["console","log"],"mappings":"2BAAAA,QAAAC,IAAA","file":"bundle.js","sourcesContent":["console.log(123);\n"],"sourceRoot":""}

image

Each of these mappings are mapped to the first column, which goes back to what @september28 said earlier. We should look closer at how tokens are mapped to columns inside Terser. I thought Terser might have trouble with composing source maps, but I ruled that out when this change to the Terser plugin still presented the error:

image

@connorjclark

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

This is suspicious:

image

This method is called by do_add_mapping, and attempts to translate the line/col to the input source map domain. Reminder that the input source looks like this:

window.blah = [
/* 0 */
/***/ (function(module, exports) {

console.log(123);


/***/ })
/******/ ]

And the map that accompanies it is correct:

{
  "version": 3,
  "sources": [
    "/Users/cjamcl/src/me/webpack-sourcemap-issue/index.js"
  ],
  "names": [],
  "mappings": ";;;;AAAA",
  "file": "x",
  "sourcesContent": [
    "console.log(123);\n"
  ]
}

So why can't Terser find line 5 column 8? I can - look!

image

This brings me to, what I hope, is our final stop on the River Styx (please allow me to be literary, source maps are driving me crazy). I set a breakpoint here:

image

and dig into BasicSourceMapConsumer.prototype.originalPositionFor, which leads me to the lazily generated property __generatedMappings, created via BasicSourceMapConsumer.prototype._parseMappings. _parseMappings is where I think we'll find our answer, but it requires knowledge of the arcane mapping format.

...or does it? We've come this far, and (well, at least for me) with very little understanding of how source maps actually work. Why learn now?

Ok I actually did learn a bit. __generatedMappings ends up with just one mapping in it, ...
;;;;AAAA corresponds to skipping the first few lines (each ;), and the AAAA is (watering this down a lot, I'm sure) and "running offset index" (I made this up) for each component of a mapping - source file, generated column, original line, original column, and name (note that generated line comes from tracking all those ;). Different amounts of components can be decoded in each segment. Here, there are just 4, so name is not accounted for (that's fine). Each letter is the base64 encoded offset to apply to the previously seen value of that component. Here, it's all zeros ... hence why we get zero for the column. The other zeros make sense (there's just one source, and only one line in the original source of console.log(123)). But not column.

So now ... let's figure out why the map created via MainTemplate.js seems to not have column information. Maybe that's the last stop in hell...

@september28

This comment has been minimized.

Copy link

commented Jun 12, 2019

Hmm... Some more random musings:
Line 30 of webpack-sources/lib/LineToLineMappedSource.js looks highly suspicious...

return new SourceNode(idx + 1, 0, name, (line + (idx != lines.length - 1 ? "\n" : "")));

Since the second arg of SourceNode "class" definition is aColumn, the above will be setting the column manually to 0.

similar code exists in webpack-sources/lib/OriginalSource.js around line 39:

				if(options.columns === false) {
					var content = line + (idx != lines.length - 1 ? "\n" : "");
					return new SourceNode(idx + 1, 0, name, content);
				}

However as you can see, it is protected with a conditional. I have done a check and in my case options.column is always undefined so the conditional should mean that this block is "skipped".

The code below that (in the same file) however seems to more often than not return a pos of "0" (i've added console.log statements at various points in this block), which leads me to think that something isn't quite right there...

return new SourceNode(null, null, null,
					_splitCode(line + (idx != lines.length - 1 ? "\n" : "")).map(function(item) {
						if(/^\s*$/.test(item)) {
							pos += item.length;
							return item;
						}
						pos += item.length;
						return new SourceNode(idx + 1, pos, name, item);
					})
				);

There is also a file webpack-sources/lib/ReplaceSource.js which seems to create a few new SourceNode's but doesn't seem to be so explicit in defining column values... May be worth a look though.

@september28

This comment has been minimized.

Copy link

commented Jun 13, 2019

Is it me, or is there nowhere in webpack-sources/lib that actually "parses" the source nodes code for variable names etc.?
From what I can see, the general method is to loop line by line and create SourceNodes (there are a variety of different types e.g. ConcatSource, ReplaceSource, CachedSource etc.) - I can't see anywhere these nodes are actually parsed for variable names, and therefore column information? So in most code I am debugging, the line incrementors increment, but the column variables stay at 0....

@connorjclark

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2019

Is it me, or is there nowhere in webpack-sources/lib that actually "parses" the source nodes code for variable names etc.?

Yes @september28, that's what I'm seeing too. I wonder if this is a mismatch of what level of detail is expected of an input source map to terser. It may be odd to do a token-to-token mapping (instead of the current line-to-line) for such simple transformations...

@connorjclark

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2019

BTW, although the discussion so far has just been using Webpack's bootstrap code as examples, I've just confirmed these same "off by a few" errors occur for the names inside each module source file too.

@september28

This comment has been minimized.

Copy link

commented Jun 28, 2019

Any update on this recently?

@connorjclark

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2019

I think this applies: mozilla/source-map#216

The nomenclature the source-map folks use is "granularity".

A useful next step would be to increase the granularity of the "identity map" that webpack uses (LineToLineMappedSource w/e _splitCode does...) by instead mapping token to token, and see what happens. If that works, neat! Until the source-map folk make progress on the algorithm discussed in the linked issue, that would be the fix.

@connorjclark

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2019

Changing _splitCode to this works:

var SPLIT_REGEX = /([^\d\w]+)/g;

function _splitCode(code) {
	return code.split(SPLIT_REGEX);
}

No validation errors. See how pretty it is:

image

@connorjclark

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2019

@september28 give your thoughts on webpack/webpack-sources#51 when you get a chance? :)

@sokra

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

Could you check if upgrading webpack-sources to 1.4.1 and terser-webpack-plugin to 1.4.1 helps with this issue?

@connorjclark

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

It works when there's just some simple code in the entry file, but seems to break for any require.

Test repo: https://github.com/connorjclark/webpack-sourcmap-validator-names-test
yarn test

AssertionError [ERR_ASSERTION]: Got unwanted exception: The sourcemap is not valid
Actual message: "Warning: mismatched names
Expected: __webpack_require__ || '__webpack_require__' || '__webpack_require__' || "__webpack_require__" || "__webpack_require__"
Got: require('url'); || require('url'); || require('url'); || require('url'); || require('url');
Original Line: const url = require('url');
Mapping: 1:928→1:12 "__webpack_require__" in webpack:///main.js"
@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

/cc @sokra

@ford04

This comment has been minimized.

Copy link

commented Aug 15, 2019

OK, I tried to use latest webpack version and debug with sourcemaps in production mode.

"webpack": "^4.39.2",
"webpack-cli": "^3.3.6"

webpack.config.js:

module.exports = {
  mode: "production",
  devtool: "source-map",
 ...
};

But in my case, breakpoints are still set incorrectly (IDE is VS Code). If I set mode: "development", everything works as expected. Have a look at this sample repository. Did I miss something or is there still an issue with sourcemaps?

@evilebottnawi evilebottnawi reopened this Aug 15, 2019

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Aug 15, 2019

/cc @sokra looks problem still exists 😞

@sokra

This comment has been minimized.

Copy link
Member

commented Aug 15, 2019

@ford04 This behavior seem to be correct, as these function were inlined, so the line you want to set a breakpoint on doesn't exist in the production optimized code:

image

Your code get optimized to this:

"use strict";
o.r(t);
const r = () => {
	setTimeout(() => {
		console.log("Hello arrow");
	}, 1e3);
};
console.log("Hello world"),
r(),
console.log("got me.");

@sokra sokra closed this Aug 15, 2019

@connorjclark

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2019

@ford04 unfortunately any non trivial code transformations (read: inlining) don't get mapped well. Blame the spec on that one :/

@ford04

This comment has been minimized.

Copy link

commented Aug 15, 2019

Honestly, in terms of developer/debugging experience, this behavior is very confusing, as you don't know which code parts have been optimized and where you can set your breakpoint after all. No experience on source map processing, but I would expect that my source code line 15, column 1 in index.ts (where breakpoint resides), gets mapped to the start of the inlined function (line 8 or so, column xx pointing at console.log("got me."). Instead it switches to L10 console.log("Hello arrow");.

Update:

thanks @connorjclark for the info. Didn't know that the specs are so vague. So there is no solution to this? Would mean to me, that sourcemaps in production are nonsense.

@connorjclark

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2019

So there is no solution to this? Would mean to me, that sourcemaps in production are non-sense.

I don't know if it's impossible (I'm not very familiar with uglifyjs's codemods), but at the very least it's a combination of insufficient spec + practicality.

Would mean to me, that sourcemaps in production are nonsense.

I wouldn't say that. Yea, some places don't get mapped well, but the majority does. I know that in Chrome DevTools, the unmappable lines are faded (last I checked, that was not the case in FF DevTools) - which does help a lot. If you attempt to set a breakpoint on such a line in Chrome DevTools, we do a best-attempt at placing it elsewhere.

EDIT: You may be interested in some notes of mine re: source maps and their limitations.

@ford04

This comment has been minimized.

Copy link

commented Aug 15, 2019

Thank you! I'll have a look at it. At least good to know that now sourcemaps are working as intended (or as accurate as the spec permits).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.