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

Allow options to be passed to the javascript compressor #1500

Closed
wants to merge 1 commit into from

Conversation

nathansobo
Copy link

I need to pass the :except option into uglifier to protect the $super keyword in my javascript. I checked the API's of YUI and Closure, and all 3 support an options hash passed to the initializer. So it seems like another config key addresses the issue.

@josevalim
Copy link
Contributor

Maybe we could allow js_compressor to be a lambda and in such cases we just invoke it? Wouldn't it be better than adding a new config option? /cc @wycats

@nathansobo
Copy link
Author

That seems reasonable to me, too.

@neerajsingh0101
Copy link

@nathansobo what is so special about $super in JavaScript. And secondly why uglifier does not handle it correcly already? I'm just curious!

@nathansobo
Copy link
Author

We built a layer on top of normal prototype-based inheritance that allows you to specify $super as the first parameter to a method and call it to invoke the method from the superclass. Prototype.js uses a similar technique as well. So the uglifier can't know we're planning on using the parameters in this way and we have to tell it. Our module system actually inspects the names of the function parameters when it's setting up our constructors.

@neerajsingh0101
Copy link

Wow. That's hardcore. Thanks for the clarification.

jake3030 pushed a commit to jake3030/rails that referenced this pull request Jun 28, 2011
…rails#1500 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
@jasonmp85
Copy link

So pull request thread hijacking aside, is this feature going anywhere? Is it in 3.2, maybe? I kind of need it, too.

@carlosantoniodasilva
Copy link
Member

@nathansobo any idea to do more work on this issue, as per @josevalim idea? Otherwise I'd like to close the issue, thanks.

@nathansobo
Copy link
Author

I liked the lambda idea better than my pull request, but I don't have time to implement it right now. I'll go ahead and close it for now.

@nathansobo nathansobo closed this Apr 28, 2012
@carlosantoniodasilva
Copy link
Member

@nathansobo alright, thanks.

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

Successfully merging this pull request may close these issues.

None yet

5 participants