leshill / handlebars_assets

Use handlebars.js templates with the Rails asset pipeline.
MIT License
648 stars 159 forks source link

Resister engine using `Sprockets` vs `app.assets`. #70

Closed metaskills closed 11 years ago

metaskills commented 11 years ago

In rare cases with other eninges mounted, using app.assets sprockets engine instance causes a can't modify immutable index errors like this one below when compiling assets.

** Execute assets:environment
** Invoke environment (first_time)
** Execute environment
rake aborted!
can't modify immutable index
/Users/kencollins/.rbenv/versions/1.9.3-p392/lib/ruby/gems/1.9.1/gems/sprockets-2.2.2/lib/sprockets/index.rb:80:in `expire_index!'
/Users/kencollins/.rbenv/versions/1.9.3-p392/lib/ruby/gems/1.9.1/gems/sprockets-2.2.2/lib/sprockets/processing.rb:32:in `register_engine'
/Users/kencollins/.rbenv/versions/1.9.3-p392lib/ruby/gems/1.9.1/gems/handlebars_assets-0.12.1/lib/handlebars_assets/engine.rb:5:in `block in <class:Engine>'

I have found that using register_engine with Sprockets solves the issue. Even tho I have authored a few gems like less-rails, I am uncertain why this is the case and how the issues really manifested. I can say that it appears that using Sprockets.register_engine vs app.assets.register_engine is the preferred way, see searches below.

https://github.com/search?q=Sprockets.register_engine&ref=cmdform&type=Code https://github.com/search?q=app.assets.register_engine&ref=cmdform&type=Code

If required, I would be more than willing to investigate why this may be the best way to register a template engine. For now, I only made it as far as reading the Sprockets::Index comments and where the specific exception was raised.

leshill commented 11 years ago

Hi @metaskills,

Thanks!

metaskills commented 11 years ago

Thanks Les!

For anyone watching this issue, I noticed Les was nice enough to put this into a v0.14.0 release. Cheers!

abhishiv commented 11 years ago

I don't have time to put out a detailed bug report, but this commit seems to break my 3.2.13 installation. I get the HandlebarsTemplates is not defined in js console.

For now I have just rolled back to 0.13.0

leshill commented 11 years ago

Hi @abhishiv,

Any details would be greatly appreciated, I will take a look at this later today.

metaskills commented 11 years ago

I can help too if needed. @abhishiv, do you have a stack trace?

abhishiv commented 11 years ago

Hey guys, there's no stacktrace. The .hbs templates just seem to be ignored. So i just have Uncaught ReferenceError: HandlebarsTemplates is not defined in the browser.

I have the following in my Gemfile

gem 'rails', '3.2.11'
gem 'handlebars_assets', "0.14.0"

If it's not reproducible let me know. I'll dig more into it.

metaskills commented 11 years ago

Hey guys, there's no stacktrace

Whoops :)

So I can confirm this happens and a potential fix, register the template handler early on as possible. In less-rails, I do that in the before_initialize hook. I have confirmed that this implementation for engine.rb works all around.

module HandlebarsAssets
  class Engine < ::Rails::Engine

    config.before_initialize do |app|
      next unless app.assets
      Sprockets.register_engine('.hbs', TiltHandlebars)
      Sprockets.register_engine('.handlebars', TiltHandlebars)
      Sprockets.register_engine('.hamlbars', TiltHandlebars) if HandlebarsAssets::Config.haml_available?
      Sprockets.register_engine('.slimbars', TiltHandlebars) if HandlebarsAssets::Config.slim_available?
    end

  end
end
leshill commented 11 years ago

Hi @metaskills,

Glad you found it, please make a new PR and I will pull that in.

metaskills commented 11 years ago

Will do... there is one small error in that too.

metaskills commented 11 years ago

Done in #73

abhishiv commented 11 years ago

Great, i'll check it once it's pulled in.

I have a feeling this should fix a similar issue I have been having with maccman/catapult and phonegap.

Thanks

leshill commented 11 years ago

This has been rolled back and a new gem version has been pushed so that folks automatically pulling them in get the new version.