map.sources converted to absolute paths + other changes

[Volune]

Motivation:
Webpack generates useless paths in sourcemaps when the original sourcemap contains relative paths. For example {"sources":["../../src/MyComponent.js"]} will generate webpack://../../src/MyComponent.js

This PR contains the following changes:

• The sources in the returned map are now converted to absolute paths. I added an option keepRelativeSources to prevent this behavior. The behavior is enabled by default because it improves the source map generation by webpack.
• The loader doesn't use webpack's this.resolve to resolve the sources path. The reason is that sources are URIs (or paths) not meant to be resolved by webpack. (The first reason was resolve fails if the file doesn't actually exist)
• The loader will try to load sources from disk when the sourcesContent value is null
• Some code "cleaning"/"refactoring": use of Promises internally, remove deprecated new Buffer, add .editorconfig for tabs

---

By the way, I've got these suggestions while working on this PR:

• Prevent reading SourceMap/Sources from files not in the same npm package as the resource, by default. (prevent things like //#sourceMappingURL=/etc/passwd)
• Add an option to force URLs in the map's sources list, for Explore using urls instead of absolute paths in source-map 'sources' array webpack/webpack#8226

[jsf-clabot]

All committers have signed the CLA.

[mlavina]

@michael-ciniawsky sorry for tagging if this is incorrect, but you seem to be the maintainer of this library. Any chance you can take a look at this? I think this would also solve the issue with #51

[mlavina]

@d3viant0ne Any chance you can take a look at this PR?

[alexander-akait]

@mlavina it is invalid fix, you should use this.resolve because it is allow webpack use own resolve logic https://webpack.js.org/concepts/module-resolution/

[Volune]

@mlavina it is invalid fix, you should use this.resolve because it is allow webpack use own resolve logic https://webpack.js.org/concepts/module-resolution/

From my understanding source maps are not modules, are source map urls are not meant to follow webpack's logic. Can you explain the benefits of using this.resolve?

[bywo]

I think this is a nice improvement and would like to see this merged! I'm currently running into this issue: #51

[justingrant]

@Volune - I found this PR after struggling with multiple different libraries emitting bogus relative paths pointing to source files that didn't exist. I have some concerns about this PR in the context of what I learned about these "missing source" cases.

TL;DR - there are three problems I've seen:

1. webpack emits relative paths in sourcemaps, which means that later stages in the build pipeline need to "un-relative" those paths before putting them into the bundled sourcemap, typically using a devtoolModuleFilenameTemplate function that merges paths and makes them into either absolute paths or relative to the app root.
2. some libraries don't ship their source when they publish to npm, so their sourcemaps point to nothing at runtime
3. some libraries have paths in sourcemaps that exist at build time, but at runtime the source exists at a different path. The most common case is monorepos where at build time the source of dependency packages live in sibling directories (e.g. /foo and /bar) but at runtime relative paths (e.g. ../foo/src) won't work because the source at runtime is inside another npm package.

I think your PR will do a great job making (1) above better. It will simplify making build pipelines that digest existing sourcemaps.

But one nice thing about relative paths is that detecting problems (2) and (3) is easy: just look for '../' in the final "dependency bundle" sourcemap. If you see any, it's likely to be missing at runtime. Relative paths that pointed to actual files would have been removed by the custom devtoolModuleFilenameTemplate function in the top-level build step.

But without any relative paths, it will be less obvious which paths have failed to be found. Do you have any ideas for how to identify cases (2) and (3) if all sourcemap paths are non-relative?

[Volune]

@justingrant I'm not sure I understand all of your cases.

For (2) source-map-loader already has a warning if it cannot find the source map. One can either add an exclude rule for that package or make a PR to ship the source map.

For (3)

• What do you call "runtime" and "build time". For me, source-map-loader is used at build time.
• Do have have an example of this monorepo use case?
• If you mean that webpack's input, the source map are in another package and cannot be resolved, then same as (2)
• If you mean that webpack's output, the bundled source maps will be shipped in another package, that's outside of the scope of any loader anyway.

[justingrant]

Hi @Volune, thanks for the quick reply!

What do you call "runtime" and "build time"? For me, source-map-loader is used at build time.

Sorry , I was imprecise. "build time" === library build time (first bundling). "run time" === app build time (bundle of library bundles). I think of it as runtime because most of my pipelines have webpack bundling integrated into running the app in development mode.

Do have have an example of this monorepo use case?

Yep. I've seen a few examples recently, but the worst offender I've seen is https://github.com/aws-amplify/amplify-js. Here's the sourcemap: https://unpkg.com/aws-amplify@1.1.22/dist/aws-amplify.js.map. You can see the relative paths (grep for ..) that are not found when I integrate this library into my app's webpack bundle.

If you mean that webpack's input, the source map are in another package and cannot be resolved, then same as (2)
If you mean that webpack's output, the bundled source maps will be shipped in another package, that's outside of the scope of any loader anyway.

Using amplify-js as an example, its Lerna pipeline builds 10-ish packages from the same set of sibling folders that are symlinked to node_modules. For example, the amplify-js package depends on the @aws-amplify/core package. At build time, these packages are siblings in packages/amplify-js and packages/core, respectively. But when someone installs amplify-js from npm, the packages are no longer siblings (./node_modules/amplify-js vs. ./node_modules/@aws-amplify/core so the relative links in the sourcemap (e.g. webpack:///../core/lib/StorageHelper/index.js) will fail to resolve.

I'm planning to do a PR to help them fix this, once I figure out exactly how to fix it.

For (2) source-map-loader already has a warning if it cannot find the source map.

This is helpful, I didn't know about this warning. Is there a webpack config setting needed to show it?
Most of my webpack work is on top of serverless framework, so I'll investigate to see if serverless is swallowing the warning. The other build-on-webpack tool I'm familiar with is create-react-app, which definitely swallows this warning along with all other warnings too AFAIK.

I think the real way to fix (2) is to add a warning into the output of npm publish, because then library authors can fix it before publishing instead of the current state where library users like me must find and fix problems after the fact (e.g. floating-ui/floating-ui#761).

[justingrant]

@Volune - I've got some feedback after using your fork in a few projects. First, this is a huge improvement! It solved almost all of the sourcemap issues in my react app. I'm also seeing sourcemap warnings for the first time, which is very helpful in tracking down problematic dependencies. I assume that the old source-map-loader code wasn't even digging into the sourcemaps deep enough to find those missing files, which is why I wasn't seeing the warnings before. Also, now that I'm seeing warnings, most of my earlier concerns are moot. Thanks so much for building this!

Also, with the help of your fork, I was finally able to PR the problematic aws-amplify package to fix most of its sourcemap issues. See aws-amplify/amplify-js#3059. This library still emits a few warnings (see minimal test repo: https://github.com/justingrant/sourcemap-test-relative), but once the questions/issues below are resolved I can PR to fix those warnings too.

I've got a few questions/concerns:

1. webpack:/// URL scheme

If a sourcemap has URLs using this scheme, your fork will throw a warning, e.g.

WARNING in ./node_modules/@aws-amplify/ui/dist/aws-amplify-ui.js
Module Warning (from ./node_modules/source-map-loader/index.js):
Cannot find source file 'webpack:///./src/index.js': URL scheme not supported
 @ ./node_modules/aws-amplify/lib/index.js 56:11-37
 @ ./src/index.js

WARNING in ./node_modules/@aws-amplify/ui/dist/aws-amplify-ui.js
Module Warning (from ./node_modules/source-map-loader/index.js):
Cannot find source file 'webpack:///webpack/bootstrap 42ac220689b741ab7f2b': URL scheme not supported
 @ ./node_modules/aws-amplify/lib/index.js 56:11-37
 @ ./src/index.js

WARNING in ./node_modules/@aws-amplify/ui/dist/aws-amplify-ui.js
Module Warning (from ./node_modules/source-map-loader/index.js):
Cannot find source file 'webpack:///webpack/universalModuleDefinition': URL scheme not supported
 @ ./node_modules/aws-amplify/lib/index.js 56:11-37
 @ ./src/index.js

AFAIK, by default webpack will emit a webpack:/// URL in sourcemaps' sources array. How should source-map-loader handle those URLs if it sees them? Seems like there are a few options for handling it:
a) throw a warning (current behavior)
b) treat it as a relative file URL relative to the current .map file being parsed
c) treat it as a relative file URL relative to the root of the current package being built. (cwd? folder where webpack.config.js is? folder where package.json lives?)
d) if it starts with ./ then (c), else (b).

Unfortunately I'm not aware of a standard or documentation for the webpack:// URL scheme, so I'm not exactly sure what webpack:///./ is expected to mean, especially in context of a dependency library that's being bundled with a parent app.

2. Unusual URLs from Webpack

Webpack itself seems to emit weird, non-standard relative URLs in some sourcemaps, e.g.

"(webpack)/buildin/global.js",
"(webpack)/buildin/module.js",
"fs (ignored)",
"webpack/bootstrap",
"webpack/bootstrap 1e9fdad6b58b6d503dfe",
"webpack/universalModuleDefinition",

How should these be handled by source-map-loader? Currently, these throw URL-scheme warnings in your code because the sourcemaps I have all prepend the webpack:/// prefix to the relative URLs above, but even if we solve the URL scheme issue, I assume we'd run into other errors downstream because they are not real file paths. Given that webpack itself is creating these, I assume we should not throw warnings for them. What do you think?

3. source-map-resolve

Today I ran across this package: https://github.com/lydell/source-map-resolve which is used by the snapdragon parser. Does this overlap with what you've built here? Is there anything useful in that package that could be re-used here?

[justingrant]

@Volune - any thoughts re: the questions in my last post? I'm eager to fix the last remaining sourcemap issues in my dependencies but wanted to get your advice first.

[Volune]

@justingrant I haven't worked on this in a while, so I don't remember everything.

In my point of view, source-map-loader is used primarily to load the source map of a library for webpack to build an application (or a library to be loaded directly from the browser). If you use webpack to build a library to load in another webpack build, you lose most of the benefit of tree shaking.
From this point of view, there should not be a case where source-map-loader encounters a webpack:// url.

If you still use webpack library in a webpack build, here is my opinion for the different cases:

• If the source of the library is provided in a separate file, the source map must use a relative path to the source.
  (Using a relative path outside of the module, or an absolute path, would raise security concerns. I don't want to go into these details here)
• If the source is provided in the sourcesContent field of the source map, regardless of the format of the source url (could be relative path, webpack:// or whatever), the loader should pass it unmodified to webpack.
  I don't think a warning is required in this case.
  I also don't think that "how webpack should handle it" is a concern for this specific loader. Other loaders may provide similar source maps to webpack.
• If the source is not provided, and the source url is not a relative url, we should emit a warning. We don't want to break the webpack build just because of a missing source.

In short for 1. and 2.: either it's in sourcesContent or emit a warning.
For 3.: It seems to overlap, but also seems not updated in a year. I don't have time to investigate more.

Hope that helps.

[niksy]

FWIW, applying these changes makes source maps from node_modules packages map properly. What can we help with to get this merged?

[justingrant]

Hi @bebraw @d3viant0ne @SpaceK33z @TheLarkInn - This PR is very helpful and it's been waiting for a maintainer review for 8 months. How can we help it get merged? FWIW, I've been using this fork for 3 months in a customized create-react-app config and it's worked great.

[shogunpurple]

Any update on this? It would be great to see this loader going into CRA and this PR is blocking that effort. Thanks!

[aruniverse]

May 2020, just wanted to see if there is any update on this.

[alexander-akait]

Fixed in the master

[justingrant]

Whoo-hoo! Thanks @evilebottnawi! This will unblock using source-map-loader in create-react-app which will help a lot of developers.

[alexander-akait]

@justingrant 👍 ETA is the next week