Bugfix/invalid sourcemaps whitespace

[connorjclark]

Fixes https://github.com/webpack/webpack/issues/7616

OriginalSource created a source map that failed validation (sourcemap-validator). The pos variable tracks the column number for a particular line, but doesn't get incremented when the current token is all whitespace. I believe that only happened for leading whitespace, as all (but possibly the last) token returned by _splitCode would contain some non-whitespace character. The effect was off-by-some-columns errors in the mappings, which manifested in the bootstrap template for webpack (it has leading whitespace).

To create a test, I had to expand the interface for ReplaceSource.replace to get names populated in the source map.

[alexander-akait]

@Hoten CI test failed, need fix

[connorjclark]

Pushed fixes

[codecov[bot]]

Codecov Report

Merging #41 into master will increase coverage by 0.14%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #41      +/-   ##
==========================================
+ Coverage   75.04%   75.19%   +0.14%     
==========================================
  Files          11       11              
  Lines         505      508       +3     
  Branches       83       83              
==========================================
+ Hits          379      382       +3     
  Misses        126      126

Impacted Files	Coverage Δ
lib/OriginalSource.js	96.61% <100%> (+0.11%)	:arrow_up:
lib/ReplaceSource.js	95.87% <100%> (+0.02%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 30d2a80...72a3594. Read the comment docs.

[codecov[bot]]

Codecov Report

Merging #41 into master will increase coverage by 0.48%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #41      +/-   ##
==========================================
+ Coverage   75.04%   75.53%   +0.48%     
==========================================
  Files          11       11              
  Lines         505      515      +10     
  Branches       83       84       +1     
==========================================
+ Hits          379      389      +10     
  Misses        126      126

Impacted Files	Coverage Δ
lib/OriginalSource.js	96.61% <100%> (+0.11%)	:arrow_up:
lib/ReplaceSource.js	96.01% <100%> (+0.16%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 30d2a80...8de1165. Read the comment docs.

[sokra]

Thanks

[dan-codes-16]

@Hoten did this PR fix the original issue? webpack@4.20.0 uses webpack-sources@1.3.0, but the issue is still reproducible in https://github.com/kenhoff/webpack-sourcemap-testing when webpack version is updated.

[connorjclark]

Yeah, I'm still seeing the same issue.

Side note, your test case isn't as intended. You've locked webpack to 4.12.2, and may be using the globally installed version for compilation (prefer npx webpack over webpack).

[dan-codes-16]

@Hoten just in case I've forked the repo and updated webpack version to the latest and changed the scripts https://github.com/adaniliuk/webpack-sourcemap-testing

There is the output:

$ webpack
Hash: 9bd50b65233535f2f320
Version: webpack 4.22.0
Time: 361ms
Built at: 2018-10-22 10:42:17
        Asset       Size  Chunks             Chunk Names
    bundle.js  984 bytes       0  [emitted]  main
bundle.js.map   4.52 KiB       0  [emitted]  main
Entrypoint main = bundle.js bundle.js.map
[0] ./src/index.js 46 bytes {0} [built]

WARNING in configuration
The 'mode' option has not been set, webpack will fallback to 'production' for this value. Set 'mode' option to 'development' or 'production' to enable defaults for each environment.
You can also set it to 'none' to disable any default behavior. Learn more: https://webpack.js.org/concepts/mode/

$ node test-sourcemaps.js
/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)

cc @sokra

[edmorley]

did this PR fix the original issue? webpack@4.20.0 uses webpack-sources@1.3.0, but the issue is still reproducible

Would you mind opening a new issue so this doesn't get lost in the closed PR comments?

[dan-codes-16]

@edmorley sure, I was going to do that. New issue - https://github.com/webpack/webpack/issues/8302 Thank you.