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 running r.js in either Node or Java #5

Merged
merged 2 commits into from
Nov 26, 2012
Merged

Allow running r.js in either Node or Java #5

merged 2 commits into from
Nov 26, 2012

Conversation

jmbowman
Copy link

I added a configuration option and some minor code changes to allow using Node instead of Java to run the optimizer. In my use case making this switch reduces the duration of the run by a factor of 4. It means using UglifyJS instead of Closure Compiler, but in my case the size difference between the output of the two is negligible.

@etianen
Copy link
Owner

etianen commented Nov 13, 2012

Haha! I was going to add this myself, but you beat me to it! I've got a project that uses a loader plugin I wrote to inline base64 representations of images, and on my quad-core laptop the compilation step takes around 5 minutes! This will be a welcome addition.

The only thing I'd ask, would be if you could change the setting from REQUIRE_NODE to REQUIRE_ENVIRONMENT, which can be "node" or "rhino", defaulting to "rhino". That would match the equivalent settings used internally by NodeJS.

Also, a line in the README describing he setting would be good.

Thanks for your input! :D

@jmbowman
Copy link
Author

Anything in particular still holding back the merge, or just haven't had time to work on django-require recently?

etianen added a commit that referenced this pull request Nov 26, 2012
Allow running r.js in either Node or Java
@etianen etianen merged commit 77dbff7 into etianen:master Nov 26, 2012
@etianen
Copy link
Owner

etianen commented Nov 26, 2012

Nothing at all (other than GitHub never told me you'd made those changes 13 days ago).

Thanks!

@etianen
Copy link
Owner

etianen commented Nov 26, 2012

So, I'd love to hear your opinions on issues #6 and #7, particularly #7

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

2 participants