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

🐛 bug report: parcel does not load existing sourcemaps of pre-transformed JS assets #741

Closed
lxe opened this issue Feb 4, 2018 · 12 comments

Comments

@lxe
Copy link

lxe commented Feb 4, 2018

This is a 🐛 bug report

General summary:

Importing/requiring a pre-built file does not load its sourcemaps.

For example, mapbox-gl-js exports a pre-packaged main which lives in node_modules/mapbox-gl/dist/mapbox-gl.js. It also contains a sourceMappingUrl //# sourceMappingURL=mapbox-gl.js.map. There is a mapbox-gl.js.map in dist/ as well.

When doing import mapboxgl from 'mapbox-gl' , parcel correctly bundles node_modules/mapbox-gl/dist/mapbox-gl.js, but does not load and re-process its sourcemap.

I haven't gotten far with debugging it, but it seems that that file does not satisfy any of the conditions that would mark it for transformation (https://github.com/parcel-bundler/parcel/blob/master/src/transforms/babel.js#L29).

Artificially marking it for a transform (by hardcoding something like if (/mapbox-gl\.js/.test(asset.name)) return true will run the babel compiler on it, which is also not what we want.

To reproduce:

yarn add mapbox-gl

Inspect the entrypoint:

❯ jq .main node_modules/mapbox-gl/package.json
"dist/mapbox-gl.js"

Inspect the presence of the entrypoint and the map:

❯ ls node_modules/mapbox-gl/dist/
mapbox-gl-dev.js mapbox-gl.css    mapbox-gl.js     mapbox-gl.js.map svg

Looks like we're pointing to the sourcemap correctly:

❯ ag "sourceMapping"  node_modules/mapbox-gl/dist/mapbox-gl.js
566://# sourceMappingURL=mapbox-gl.js.map

Import the file, and package it with parcel:

// index.js
import mapbox from 'mapbox-gl`

// index.html
<html><body><script src="./index.js"></script></body></html>
❯ parcel index.html
Server running at http://localhost:64815
✨  Built in 1.88s.

Verify that the sourcemap of mapbox-gl.js does not get repackaged:

🌍 Your Environment

Software Version(s)
Parcel 1.5.1
Node v8.9.3
npm/Yarn 1.3.2
Operating System Darwin
@lxe lxe changed the title 🐛 bug report: parcel does not load sourcemaps for JS assets not marked for transformation. 🐛 bug report: parcel does not load sourcemaps of pre-transformed JS assets not marked for transformation. Feb 4, 2018
@lxe lxe changed the title 🐛 bug report: parcel does not load sourcemaps of pre-transformed JS assets not marked for transformation. 🐛 bug report: parcel does not load existing sourcemaps of pre-transformed JS assets Feb 4, 2018
@lxe
Copy link
Author

lxe commented Feb 4, 2018

Basically we need something like https://github.com/webpack-contrib/source-map-loader

@Mickyfen17
Copy link

Mickyfen17 commented Feb 25, 2018

I've just come across this same issue today when trying to use mapbox-gl & parcel together.

@devongovett
Copy link
Member

This seems easy enough to do. We already have the infrastructure in place, just need to parse the existing source map url comment in the code and load it.

@Offirmo
Copy link

Offirmo commented May 31, 2018

Tried parcel, was delighted, then put a "debugger" statement...

Please fix this!

@snstrauss
Copy link

I agree.
Using a 'debugger;' statement causes the debug session to pause at random places across the code.

What can I do about it?

@lxe
Copy link
Author

lxe commented Jun 20, 2018

What does debugger; have to do with this issue? @Offirmo @snstrauss Did you mean to comment elsewhere?

@Offirmo
Copy link

Offirmo commented Jun 21, 2018

@lxe think about it...

[edit] using "debugger" make the source map issue apparent by showing code mismatched to where the debugger clause is.

@iFwu
Copy link

iFwu commented Jan 11, 2019

any updates?

@medmin
Copy link

medmin commented Jul 9, 2020

any updates ?

@Bug-Reaper
Copy link

Any insight into how to debug issues where parcel fails to load existing sourcemaps. Similar problem with this npm module: https://github.com/mfbx9da4/deep-email-validator

@lemraus
Copy link

lemraus commented Oct 12, 2020

I am willing to re-open this issue as I have come across this problem earlier today, and after taking a look at #1349 I understood where the problem is. #1349 did not entirely solve this issue, below is an explanation as to why.

From the source code of #1349, in file src/assets/JSAsset.js, line 26:

const SOURCEMAP_RE = /\/\/\s*[@#]\s*sourceMappingURL\s*=\s*([^\s]+)/;

Then later, line 114:

let match = this.contents.match(SOURCEMAP_RE);

This would work fine if all sourceMappingURL statements started with //.
I have recently started to work with neovis.js, a client visualization tool for Neo4j databases. Now take a look at their minified neovis.js file, line 95, column 21247:

o&&btoa&&(i+="\n/*# sourceMappingURL=data:application/json;base64,".concat(btoa(unescape(encodeURIComponent(JSON.stringify(o))))," */"))

Uh-oh, looks like folks at neovis.js felt like using multiline comments (/* */) instead of the usual // for sourceMappingURL.
I started thinking that it might be a neovis.js related issue, however it turns out things work fine with webpack.

After looking at the source code of parcel v2, it seems that the regexp has been corrected since v1. In file packages/core/utils/src/sourcemap.js, line 7:

export const SOURCEMAP_RE: RegExp = /(?:\/\*|\/\/)\s*[@#]\s*sourceMappingURL\s*=\s*([^\s*]+)(?:\s*\*\/)?\s*$/;

It remains a problem as installing parcel with a simple yarn add parcel still defaults to v1.
For now a temporary fix would be to upgrade to v2, but using the @next version of a package is not an acceptable fix, IMHO.

I think this could be solved by simply copying the regexp from v2 to the v1 source code, should I file a pull request with the changes?

@jzohrab
Copy link

jzohrab commented Apr 6, 2021

Hi all, I'm running into this now (parcel version 1.12.5), with a js library called Vexflow. It has the same commenting style noted above:

o&&btoa&&(n+="\n/*# sourceMappingURL=data:application/json;base64,".concat(btoa(unescape(encodeURIComponent(JSON.stringify(o))))," */"))

@lemraus I'm happy to create and submit this PR with your suggested regex above, but I don't see an obvious v1 branch or tag to work off of. @devongovett - any suggestions? Thanks!


edit: actually, it might be trickier than I expected. My node_modules/parcel-bundler/src/utils/loadSourceMap.js already had const SOURCEMAP_RE = /(?:\/\*|\/\/)\s*[@#]\s*sourceMappingURL\s*=\s*([^\s*]+)(?:\s*\*\/)?/;, so I added the error details to the try/catch:

    } catch (e) {
      logger.warn(
        `Could not load existing sourcemap of "${asset.relativeName}, error = ${e}".`
      );

and got Could not load existing sourcemap of "../node_modules/vextab/dist/div.prod.js, error = SyntaxError: Unexpected token r in JSON at position 0".

Earlier in the code, it's doing

      if (dataURLMatch) {
        filename = asset.name;
        logger.warn(`a. making buffer from ${dataURLMatch[1]}`)   // <-- added
        json = Buffer.from(dataURLMatch[1], 'base64').toString();

which is outputting

a. making buffer from ".concat(btoa(unescape(encodeURIComponent(JSON.stringify(o)))),"

So the regex may be off, at least for this particular case.

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

No branches or pull requests