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

support non-inline sourcemaps #7

Closed
sidorares opened this issue Sep 2, 2013 · 8 comments
Closed

support non-inline sourcemaps #7

sidorares opened this issue Sep 2, 2013 · 8 comments

Comments

@sidorares
Copy link

My use case: I'm trying to use spine with browserify, and resulting bundle contains multiple original non-inline sourcemap comments instead of expected one combined inline. Example compiled js and map files: spine/lib

My current workaround - preprocess generated files and convert non-inline maps to inline. I'm using https://github.com/sidorares/embed-source-map
Ideally this should be handled by combine-source-map without external preprocessing

@JonET
Copy link

JonET commented Sep 2, 2013

I'm running into a similar issue with my plug-in into the mimosa build system
https://github.com/JonET/mimosa-browserify

The non-inline input source maps get ignored. Input that uses the new sourcemap syntax (//#sourceMappingURL vs //@sourceMappingURL) is also ignored.

@thlorenz
Copy link
Owner

thlorenz commented Sep 2, 2013

I created mold-source-map to give you more power over sourcemaps.
It allows you to pull out the existing sourcemaps and do with them whatever you please, even reference an external .map file.
Please have a look at that and let me know if that helps.

Ideally this should be handled by combine-source-map without external preprocessing

The scope of combine sourcemap is limited, so converting non-inline to inline should be done in a separate module. Once that module exists we have the option to have combine-source-map call into it, but am not sure if it should even do that at all.

IMO, instead of combine-source-map having any part in that at all, that should be done by running each file through that module before it reaches combine-source-map so it can do exactly and only what it is designed for combine the source maps.

@sidorares
Copy link
Author

Then it probably should not try to attempt parsing comment at all (what about CSS maps - '/*# map */`?)

var fooFile = {
    source: '(function() {\n\n  console.log(require(\'./bar.js\'));\n\n}).call(this);\n' + '\n' + fooComment
  , sourceFile: 'foo.js'
  , map: fooMapObj // somebody else created map or extracted from comment. 
                               //  It's also somebody else's responsibility to strip comment
};
var barFile = {
    source: '(function() {\n\n  console.log(alert(\'alerts suck\'));\n\n}).call(this);\n' + '\n' + barComment
  , sourceFile: 'bar.js'
  , map: barMapObj
};

var offset = { line: 2 };
var base64 = combine
  .create('bundle.js')
  .addFile(fooFile, offset)
  .addFile(barFile, { line: offset.line + 8 })
  .base64();

@thlorenz
Copy link
Owner

thlorenz commented Sep 3, 2013

@sidorares keep in mind that this was created for source maps adhering to the source map v3 proposal.

The CSS maps you are mentioning don't look like they follow that standard.

This module is quite small and I want to keep its focus on standard source maps, so I'd suggest you create a separate module that works with CSS source maps. You probably want to start by creating a CSS source map version of convert-source-map.

@sidorares
Copy link
Author

The map itself in CSS is the same v3 standard

see page 7:

This recommendation works well for JavaScript, it is expected that other source files will have other conventions:

CSS
/*# sourceMappingURL=<url> */

My point is this module should just combine v3 maps (no matter if it's js or css), and not attempt to parse comments. There is no need for separate CSS combiner, maps format is identical to js sourcemap

@thlorenz
Copy link
Owner

thlorenz commented Sep 3, 2013

Ah, I see your problem now, the CSS sourcemap is using the newer # version.
There is a related issue. I would suggest moving this discussion there.

However so far I have been reluctant to switch over to this upgraded version since it's not implemented in enough browsers yet (Chrome even removed a warning about upgrading recently).

Additionally most source map generators still generate sourcemap comment according to the initial spec (with the @).
Hope this helps.

@thlorenz
Copy link
Owner

thlorenz commented Nov 5, 2013

The involved modules have been updated to the latest specs. I hope this fixes your problem, see here.

I'm also working on properly removing source map file comments. This change will land as a patch to convert-source-map and combine-source-map within the next day or so.

@sidorares
Copy link
Author

So with your update non-inline sourcmaps will be just ignored?

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

No branches or pull requests

3 participants