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

Creating JS SOURCE MAP + GZIP #41

Closed
wants to merge 3 commits into from
Closed

Conversation

nikitok
Copy link

@nikitok nikitok commented Nov 1, 2013

Creating JS SOURCE MAP + GZIP

@nikitok
Copy link
Author

nikitok commented Nov 1, 2013

Commit 9891ef6 can be removed)

@ntrp
Copy link

ntrp commented Nov 6, 2013

+1 for this feature. Would love to see source maps generation on this plugin!

@mckramer
Copy link

👍

Very cool, a few of my random thoughts:

  1. There is a lot of code duplication between minify and mergedAndMinify now. I think it might make more sense to expose files or change the API for minify to pass a list of source files through.
  2. When skipping merge, should a map file be created for each file? I would think that it would. (Which actually might tie back into 1.
  3. See this mailing list conversation where the standard was changed from //@ to //# for the sourceMap declaration (the //# should be deprecated in browsers). Also, some good information here.
  4. I would actually be pretty interested in this feature being enabled by default.

If I get a chance, I can mock something up, because I have a feeling this wasn't very clear.

@jlambert121
Copy link

+1 - I'd love to see this! The plugin is great, thanks for your work.

@rgba
Copy link

rgba commented Jan 27, 2014

+1 - Is there any ETA when this feature will be integrated within the minify-maven-pluigin?

@samaxes
Copy link
Owner

samaxes commented Feb 25, 2014

Hi guys,

I've created a new branch called source-maps, based on @nikitok code, where I've added support for JavaScript Source Maps.
Can I ask you to please clone this branch and help me test this feature?

@nikitok, I've commented the line options.setSourceMapLocationMappings as it seems to be redundant on the test I ran. Do you have a use case where this option is mandatory for it to work?
And thanks for your contribution!

@samaxes samaxes added this to the 1.7.3 milestone Feb 25, 2014
@samaxes samaxes self-assigned this Feb 25, 2014
@samaxes
Copy link
Owner

samaxes commented Jul 29, 2014

Merged into master.

@samaxes samaxes closed this Jul 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants