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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Webpack warns about an erroneous sourcemap path #487

Closed
ivan-aksamentov opened this issue Dec 21, 2019 · 9 comments
Closed

Webpack warns about an erroneous sourcemap path #487

ivan-aksamentov opened this issue Dec 21, 2019 · 9 comments
Labels

Comments

@ivan-aksamentov
Copy link

馃悰 Bug Report

When using immer 5.1.0 and source maps, webpack emits the following warning:

WARNING in ./node_modules/immer/dist/immer.module.js
Module Warning (from ./node_modules/source-map-loader/index.js):
(Emitted value instead of an instance of Error) Cannot find source file '../node_modules/tslib/tslib.es6.js': Error: Can't resolve '../node_modules/tslib/tslib.es6.js' in '/home/ia/projects/repro-immer-sourcemaps/node_modules/immer/dist'
 @ ./index.js 1:0-31 2:12-17
 @ multi ./index.js

Link to repro

https://github.com/ivan-aksamentov/repro-immer-sourcemaps

To Reproduce

Steps to reproduce the behavior:

git clone https://github.com/ivan-aksamentov/repro-immer-sourcemaps
cd repro-immer-sourcemaps
yarn install
yarn dev

(yarn dev is equivalent to
NODE_ENV=development BABEL_ENV=development webpack)

Observed behavior

Webpack will emit the following warning:

WARNING in ./node_modules/immer/dist/immer.module.js
Module Warning (from ./node_modules/source-map-loader/index.js):
(Emitted value instead of an instance of Error) Cannot find source file '../node_modules/tslib/tslib.es6.js': Error: Can't resolve '../node_modules/tslib/tslib.es6.js' in '/home/ia/projects/repro-immer-sourcemaps/node_modules/immer/dist'
 @ ./index.js 1:0-31 2:12-17
 @ multi ./index.js

The warning is due to incorrect sources path entry in
node_modules/immer/dist/immer.module.js.map:

{
  "version": 3,
  "file": "immer.module.js",
  "sources": [
    "../node_modules/tslib/tslib.es6.js"
  ],
  ...
}

In this case, source map assumes that tslib.es6.js is available in
../node_modules/tslib/tslib.es6.js (that is inside
<project_root>/node_modules/immer/node_modules/tslib), but it may not be the
case, for example if using yarn as a package manager, which may hoist
dependencies. In general, source maps should not make such assumptions.

Expected behavior

Webpack does not emit any warnings

Environment

"os": "Ubuntu 18.04.3 LTS",
"node": '12.14.0',
"yarn": "1.21.1",
"immer": "5.1.0"
"source-map-loader": "0.2.4",
"webpack": "4.41.4",
 

Additional information

$ git clone https://github.com/immerjs/immer
$ cd immer
$ yarn install
$ yarn why tslib
yarn why v1.21.1
[1/4] Why do we have the module "tslib"...?
...
=> Found "tslib@1.10.0"
info Reasons this module exists
   - "rollup-plugin-typescript2" depends on it
   - Hoisted from "rollup-plugin-typescript2#tslib"
info Disk size without dependencies: "60KB"
info Disk size with unique dependencies: "60KB"
info Disk size with transitive dependencies: "60KB"
info Number of shared dependencies: 0

$ cd repro-immer-sourcemaps
$ yarn why tslib     
yarn why v1.21.1
[1/4] Why do we have the module "tslib"...?
...
=> Found "tslib@1.10.0"
info Reasons this module exists
   - "webpack#chrome-trace-event" depends on it
   - Hoisted from "webpack#chrome-trace-event#tslib"
info Disk size without dependencies: "60KB"
info Disk size with unique dependencies: "60KB"
info Disk size with transitive dependencies: "60KB"
info Number of shared dependencies: 0

@justingrant
Copy link
Contributor

Unfortunately, #467 isn't the only problem with sourcemaps in v5.1.0. A more serious problem is that the sourcemap doesn't actually map immer source files. See #489 with fix in #490

Anyway, back to the bad tslib in the sourcemap. I tried to figure out how to remove tslib from the sourcemap, but haven't been successful yet. Immer's tsconfig.json sets "importHelpers": false which means that there's no runtime dependency on tslib, so it doesn't make sense to emit a reference to ../node_modules/tslib/tslib.es6.js in the sourcemap.

I think the right behavior here is simply to omit ../node_modules/tslib/tslib.es6.js from the sourcemap completely, because those TS helper functions are probably not transpiled anyway.

I'm not sure, however, who's actually injecting ../node_modules/tslib/tslib.es6.js into the sourcemap. TS compiler? rolllup-plugin-typescript2? babel? rollup? bili?

I suspect that someone more familiar with this chain of build tools might have a better idea which one is likely to be the culprit.

The only clue I found was this note below in the docs of a different rollup typescript plugin (rollup-plugin-ts) than the one that immer is using.

Typescript and tslib helpers
This plugin makes sure that the helper functions that may be emitted within the output generated by Typescript will not be duplicated across files and chunks. Instead, they will automatically be divided into chunks and imported across Rollup chunks. You don't have to do anything!

So at least in that other rollup plugin, it's doing (or at least controlling) which TS helpers are emitted where. So maybe it's rolllup-plugin-typescript2 that controls the injection of helper code... and presumably sourcemaps?

@mweststrate
Copy link
Collaborator

Should be fixed in 5.1.1. Thanks for the detailed reproduction, super helpful!

@justingrant
Copy link
Contributor

@mweststrate - I don't think that b55ef46 fixes this issue. The bogus tslib path still shows up in the sources array of the sourcemap. Excerpt here:

{"version":3,"file":"immer.module.js","sources":["../node_modules/tslib/tslib.es6.js","../src/common.ts","../src/scope.ts","../src/es5.ts","../src/proxy.ts","../src/patches.ts","../src/immer.ts","../src/index.ts"],"sourcesContent":["/*! 

I suspect that the root cause is that whoever is generating the sourcemap (per my comment above this may be TS, may be the rollup TS plugin, may be babel, may be rollup, etc.) is not properly handling the "importHelpers": false option in tsconfig.json. If importHelpers is false, then nothing for tslib should be going into the sourcemap's sources array, because there's no corresponding on-disk file for that tslib code that was injected into the transpiled source by the TS compiler.

@justingrant
Copy link
Contributor

Update: after more investigation, bili seems like the most likely culprit for tslib showing up erroneously in the sourcemap. I filed egoist/bili#280 with details and a repro.

@mweststrate
Copy link
Collaborator

@justingrant thanks for investigating and filing!

The bogus tslib path still shows up in the sources array of the sourcemap.

Yes, but since it is not used to locate the source (probably it is used for display sources, e.g. to name the files in the debugger), it is no problem.

@dkadrios
Copy link

dkadrios commented Jan 2, 2020

FWIW, I'm seeing something similar with parcel and immer 5.1.0:

鈿狅笍 Could not load source file "../node_modules/tslib/tslib.es6.js" in source map of "../node_modules/immer/dist/immer.module.js".

Hoping this goes away in 5.1.1 as well. It's just a warning, but it's ugly ;)

@justingrant
Copy link
Contributor

@dkadrios - to get rid of this warning, egoist/bili#280 will have to be fixed (or immer would have to move off of bili, but I assume the former is easier!). If you (or someone you know) is a rollup expert and could track down what's going wrong in the way bili's rollup plugins and/or configuration, then you'd have a lot of grateful fans!

I agree that the warning is a problem. In addition to ugliness, many devs prefer zero-warnings builds so that if an actual problematic issue pops up then it will be obvious and won't blend into a sea of ignored warnings.

@justingrant
Copy link
Contributor

@mweststrate - sorry for delayed response, I;m still catching up from the holidays.

it is not used to locate the source (probably it is used for display sources, e.g. to name the files in the debugger), it is no problem.

There's more to it. The sources array is used by the VSCode debugger to locate source files on disk. If a file from sources is located on disk, then VSCode will step into that file's content while debugging, will apply breakpoints to that file, etc. Only if the file isn't found will VS Code fall back to use sourcesContent (the code that's embedded inside the sourcemap file) as read-only code that's only visible while debugging.

This file-first preference exists because it makes a combined editor+debugger more useful:

  • You can set breakpoints on files even if your app is not running. This makes it easier to debug startup behavior.
  • Hot reload debugging is better because you can set a breakpoint on code that's not yet running, save the code, and have it stop on the breakpoint in the new code. (In practice there are limitations in hot reload and the chrome debugger that makes this flakey, but this workflow is great when it works)
  • You can edit the same files that you're debugging, which is kinda the point of an IDE. ;-)

Anyway, it's not a big deal but I did want to clarify that sources is for more than just display--it's needed to ensure that IDE debuggers work as expected. That's probably why parcel, webpack, etc. will complain with warnings about bogus paths in sources. Usually these warnings are a good thing because they help library maintainers fix common problems like forgetting to include the src folder when publishing to npm, bad relative paths in sources, etc.

@aleclarson
Copy link
Member

馃帀 This issue has been resolved in version 5.2.0 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants