Fixed version of gem that calculate dependensies in proper way

[egorkhmelev]

I have found that current version of gem tried to resolve and append files based on goog.require directive that actually works not so stright. For example if you will try goog.require('goog.fx'); you will got an error that file goog/fx/Animation/EventType.js (one of the dependensies) does not exist and its true, it was provided by file goog/fx/animation.js

Here is corrected version that use calcedeps.py script to calculate properly deps and include them in project. However its a bit harder coz it requires new directive. I hope there are some way to do it easier.

[igrigorik]

Hmm, how does this work in terms of performance? Last time I tried this, it was very very painfully slow when I had to shell out to python. Ideally, we'd want to just rewrite the logic in Ruby.

[egorkhmelev]

Locally it works perfect. And with sprockets default caching even better.
I'm developing now with it and don't experiencing any performance issues.

Anyway its fast fix because current version doesn't work at all with something except good.dom

Egor Khmelev Front-end and Mobile devices bookmate.com/khmelev (http://bookmate.com/khmelev)

On суббота, 4 февраля 2012 г. at 22:33, Ilya Grigorik wrote:

Hmm, how does this work in terms of performance? Last time I tried this, it was very very painfully slow when I had to shell out to python. Ideally, we'd want to just rewrite the logic in Ruby.

---

Reply to this email directly or view it on GitHub: https://github.com/igrigorik/closure-sprockets/pull/10#issuecomment-3812119

[lypanov]

It doesn't have to be fast.

My workaround for this issue several months back was to simply enable the javascript side closure deps calculation when in development mode and when in precompile mode disable and run the python.

[igrigorik]

@lypanov fair enough. I'm not a fan of forcing the python dependency, but given that the current version is effectively broken without it.. that's a strong argument to merge this in.

@hmelyoff can you please remove the .pyc files?

[lypanov]

I should have mentioned that I was using the modern builder.

IMO the namespace stuff is pretty fundamental to closure and it's only a one liner so while yes, it breaks the blog post, it enables code reuse which I suspect the existing code does not allow for. Allowing includes becomes important very fast.

[igrigorik]

I wouldn't worry about breaking the blog post - that should be the least of our concerns! The blog post can be updated at any time :)

[xrd]

@igrigorik, @egorkhmelev, @lypanov thank you for working on this issue, and @igrigorik thank you so much for this gem.

Is this pull request dead? I'm trying to use closure-sprockets and cannot get past the example on the front page. When I try to add this line to my test.js file:

goog.require('goog.ui.Button');

I get a crash when generating the list of JS files:

couldn't find file 'goog/array/arraylike' (in /Users/xrdawson/Projects/closure-rails/vendor/assets/closure-library/closure/goog/ui/component.js)

I've tried using @egorkhmelev's fork with this pull request, but then I get this issue:

NameError: uninitialized constant Sprockets::DirectiveProcessor ~/.rvm/gems/ruby-1.9.2-p290/bundler/gems/closure-sprockets-f9cdd0633736/lib/closure-sprockets/directive_processor.rb:1:in<top (required)>' ~/.rvm/gems/ruby-1.9.2-p290/bundler/gems/closure-sprockets-f9cdd0633736/lib/closure-sprockets.rb:4:in require' ...

I'm using Rails 3.2.2. I've tried using a gemset for Rails 3.1 and I get the same crash with @igrigorik's version, and "goog is not defined" from @egorkhmelev's (it does not build the closure dependency JS list at all.)

When I use @egorkhmelev's version, I do see Python being spun up in the process list, so I assume it is using the newer dependency build script.

Does anyone have a basic rails project which I can use to start using closure library inside of rails?

Thanks. Chris

[igrigorik]

I'd love to get this merged. There are a few outstanding issues in the discussion above that need to resolved first. @lypanov is this something you have bandwidth to make changes for?

[lypanov]

I'm actually using clojurescript now via clementine and no longer use closure-sprockets so alas not sorry! Maybe @egorkhmelev has time to push it on through :)

[xrd]

I will review the changed and see if I can take up from where things got left off. I am wondering if this is because I am using coffee script rather than just straight js. Thanks again everyone.

Chris Dawson Human potential, travel and connection http://webiphany.com On Mar 22, 2012 4:05 AM, "Alexander Kellett" < reply@reply.github.com> wrote:

I'm actually using clojurescript now via clementine and no longer use closure-sprockets so alas not sorry! Maybe @egorkhmelev has time to push it on through :)

---

Reply to this email directly or view it on GitHub: https://github.com/igrigorik/closure-sprockets/pull/10#issuecomment-4634106

[guilleiguaran]

@igrigorik I will be checking this later this week

[igrigorik]

@BertHartm @guilleiguaran sorry, dropped the ball.. where are we with this one?

[SpainTrain]

Fork by @egorkhmelev is working beautifully for me (https://github.com/SpainTrain/mynotes).

@igrigorik Is the main outstanding issue whether to use calcdeps or builder (or make it a config option)? I'd be willing to polish off what is left.

[igrigorik]

@SpainTrain to be honest, I've lost the thread myself as well - it's not clear what is missing. If you can confirm that what we have here works, then we can merge. If not, and you're willing to pull it over the line.. then you're officially awesome. :-)

[SpainTrain]

@igrigorik my app doesn't use some features (like soy). I'll make up a test app to confirm everything is good. Hah, I officially just needed to get my app faster! Definitely glad this was available.

[BertHartm]

@igrigorik Everything works great for me on @egorkhmelev 's fork (after the patch). speed is an issue, but not enough of one to make me care.

[igrigorik]

@egorkhmelev can you make a pull? Happy to merge it in.

[egorkhmelev]

I have made a pull 3 months ago, so you could merge

[igrigorik]

Ah, well, my bad then! Will merge in a sec.