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

RFC: Do source maps correctly #16

Closed
terinjokes opened this issue Jan 26, 2014 · 16 comments
Closed

RFC: Do source maps correctly #16

terinjokes opened this issue Jan 26, 2014 · 16 comments

Comments

@terinjokes
Copy link
Owner

As of 0.2.0 source maps are emitted as Vinyl objects on the main gulp stream. However I'm unsure if this causes issues with other downstream gulp plugins, who might not expect non-JavaScript files on the stream.

Is this a real problem actually affecting users, and if so would you be interested in source maps being emitted on a source map substream?

@terinjokes
Copy link
Owner Author

After talking with contra, I'm going to inline source maps. This avoids the issue of downstream modules assuming they're getting JS. This also allows the source maps to work without having a reference to the unminified source somewhere.

@spencerdeinum
Copy link

I don't understand how source maps can be used with v0.2.0.

It obviously creates the output source maps properly but Chrome doesn't know to use them because there is not comment added.

Am I missing something? Shouldn't uglify2 be adding that source maps comment when I have the option enabled?

@terinjokes
Copy link
Owner Author

@spencerdeinum like uglify-js (and Closure Compiler), this plugin doesn't add the source map line itself. The suggested change here would change that and inline the source map.

@spencerdeinum
Copy link

I think right now if you use the concat plugin it actually will embed the source map inside the file. (I found this out by mistake).

From what I've seen it doesn't produce valid javascript code though, I was getting errors in my file.

Do you use some other plugin to create the source map comment or do you not use them?

I know the grunt uglify plugin adds the comment but I haven't read the source to see how they do it.

@spencerdeinum
Copy link

I didn't realize this was possible with source maps, https://github.com/sidorares/embed-source-map.

Is this what you are thinking of including? I think that would be great, since we don't need to create any extra .map files anymore.

@terinjokes
Copy link
Owner Author

Different module, but same idea, yes. Already done locally, just needing to finish the unit tests.

@spencerdeinum
Copy link

Awesome!

@dbergey
Copy link

dbergey commented Feb 11, 2014

Will there be an option to keep the sourcemaps separate and inline the comment?

Sourcemaps inline-only isn't really helpful to me since the actual minified file is supposed to be as small as possible. I only want to fetch the sourcemap if necessary.

@terinjokes
Copy link
Owner Author

@dbergey: I agree, you shouldn't be serving inline source maps with production code.

Inline source maps, however, will be the general rule of transform plugins, not just this one. We'll be leaving it up to the users to hook into something like exorcist to pull them out themselves.

@aaron-cooliris
Copy link

How is this supposed to work? In the grunt world, the uglify task will concatenate multiple source files and generate a single minified file with a sourcemap comment at the end. The sourcemap will reference each of the ORIGINAL source files so that I can set breakpoints, etc. in the ORIGINAL source file. This task seems to only operate on one file at a time, implying the source files should be concated separately. But if I concat all my sources before uglification, the source map will reference only this one giant concated file artifact and not the many individual source files. I know gulp tasks are supposed to only 'do one thing', but as far as I know, there is no way to generate a proper sourcemap to original source files unless the concat is done by uglify, by passing in an array of filenames to minify()

@theefer
Copy link

theefer commented Mar 12, 2014

@aaron-cooliris To get this to work, you need the concat step to generate a source map (from N files -> 1 concatenated file), which is passed alongside the concatenated file as input to the uglify step, which generates a source map of its own (1 concatenated file -> 1 minimised file). You can then combine the source maps two get one from N files -> 1 minimised file.

@nrutman
Copy link

nrutman commented Mar 18, 2014

@theefer Have you seen any examples of this working properly? For some reason gulp-uglify is generating the following JSON:

{"version":3,"file":"app.min.js.map","sources":["app.min.js"] // [...]

I believe the file should be "app.min.js" and the sources should be "app.js" or the multiple concatenated files.

Is there an online example of how this works?

@theefer
Copy link

theefer commented Mar 18, 2014

@nrutman I don't think this feature has made it into master or any of the releases of gulp-uglify yet. @terinjokes?

But it would be possible to make this work, and AFAIK @terinjokes has been working on this.

(In another context, I've implemented working automatic chaining of source maps as part of the Plumber build tool, which has nothing to do with Gulp, but that proves the concept is viable and would like be for Gulp too.)

@terinjokes
Copy link
Owner Author

@theefer I'm working on things for all of gulp to work with source maps.

gulp-uglify master has source maps, but they're broken and I wouldn't use them.

@terinjokes
Copy link
Owner Author

Closing in favor of gulpjs/gulp#356.

@aaron-cooliris
Copy link

I made a solution that works for my project, but does not use this plugin. I have a few custom plugins/helpers: the first, a "blocks" handler, parses and extracts the block information from the HTML and pushes an object containing the block name and files onto the stream. Another helper (an object mode stream) intercepts these objects and passes the files array and target file name to a custom "uglify" helper which calls uglify.minify(), adds the sourcemap comment, and pushes the min file and the map file onto its stream. Creating a simple stream wrapper around existing NPM modules is pretty easy. If a certain gulp plugin doesn't have exactly what you need, don't be afraid to just roll your own!

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

6 participants