Nokogiri adds a lot of weight to ActionView

[iybe...@gmail.com]

Rails goes out of its way to avoid forcing an installation of bcrypt because it is a binary library.  See https://github.com/rails/rails/blob/v4.2.3/Gemfile#L21

Nokogiri forces installation of 2 binary libraries (libxml2 and libxslt), so one would expect it not to be a dependency of any of the core components of Rails.

However, starting with actionview 4.2.0, nokogiri is now a dependency.

That means every time actionview appears in a Gemfile.lock, so does nokogiri.  I would often include ActionView 4.1 in non-Rails projects just to use number_to_currency, but now with the nokogiri dependency, the overhead is hardly worth it.

Consider the fact that I'm deploying about 5 such projects to the same server, all using separate BUNDLE_PATH's.  That means 5 installations of nokogiri, none of which are being used.  This adds time to every `capistrano bundler:install` and a significant amount of disk space is wasted on this.  For any other gem, this wouldn't make much of a difference, but nokogiri is really big and takes a long time to install, and Rails has already set a precedent by not including the (much lighter) bcrypt.

Is the rails-core team open to the following solutions:

---------------

1) Separate the parts of actionview that depend on rails-dom-testing into a separate gem

Create an actionview-testing gem, which would only be necessary in the Gemfile's test group, thus saving even more overhead in production.  This would depend on action-view and rails-dom-testing, but actionview would not depend on rail-dom-testing.

(The same approach that I suggest below for rails-html-sanitizer might work for rails-dom-testing too, but it may add more complexity that carving a separate gem--there are multiple code paths that can lead you to rails-dom-testing methods, whereas there's a single method that's a bottleneck for all entries to rails-html-sanitizer.)

---------------

2) In ActionView::Helpers::SanitizeHelper, move `require "rails-html-sanitizer"` into the  #sanitizer_vendor method.

If a LoadError is raised, handle it just as we do for bcrypt: https://github.com/rails/rails/blob/v4.2.3/activemodel/lib/active_model/secure_password.rb#L60

Add rails-html-sanitizer to the Gemfile template so that it's automatically in new Rails projects.  Only upgrades and would need to manually add this to the Gemfile, so we would have to add it to the upgrade guide.  Standalone actionview projects would also need to add it to their Gemfile, but rafaelfranca has assured me that non-Rails projects should never be using rails-html-sanitizer anyway: https://github.com/rails/rails-html-sanitizer/issues/25#issuecomment-60833972.

---------------

I would love to get started on a PR.  I just need to know if it will be considered.

[Nicolás Sanguinetti]

Um, deviating a little from your particular concern (which may or may not be justified), but you really can’t complain about being forced to include a gem multiple times when you’re including action view (and thus active support, I18n, TZInfo, builder, erubis, and a couple more…) *just* to use a method that you could very well implement yourself in very simple ruby.

I mean, you were pulling like 8 or 9 gems (without counting nokogiri) just so you can avoid formatting a number a little bit, rounding it off, and prepending a “$” (or your currency of choice) to a string.

…but *then* you complain that now you have to install an extra gem?

o_O

Cheers,

-foca

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to rubyonra...@googlegroups.com.
Visit this group at http://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

[Ryan Bigg]

It’s not that they have to install an extra gem that’s the problem, I think. The problem is that Nokogiri is only used by a tiny, tiny subset of ActionView and during the course of the application that tiny subset might not be used at all.

The installation of Nokogiri can cause issues for people who are new to Rails if they’re missing the libxml or libxslt libraries. A solution is that they install those libraries. Another is that we remove the Nokogiri dependency.

I’m +1 on separating it out so that it’s an optional part of ActionView, if that’s at all possible. 

[Carlos Antonio da Silva]

As a side note, the base functionality of the number helpers was moved to Active Support, so you don't need to rely on Action View anymore in this scenario.

(I understand the concern about including nokogiri, just wanted to make sure that everyone was aware of this).

--

At.
Carlos Antonio

[iybe...@gmail.com]

Thank you for the feedback.  Thank you especially to Carlos for pointing out that I could just include ActiveSupport without ActionView.

I could have left out the personal anecdote--Ryan makes a good point that the real issue is that nokogiri should not be a dependency when it is needed by only a small subset of functionality, that often is not used at all.

So, rails-core team, are you guys on board?

[Rafael Mendonça França]

Nokogiri is needed for tests and I don't think tests are often used, at least they should. I still don't see any reason for doing this even more because almost all Rails applications use capybara and nokogiri is also a capybara dependency.

--

[Isaac Betesh]

1) By the same logic, almost all Rails applications use devise, and bcrypt is a dependency of devise.

2) capybara is not typically a dependency in production

--
You received this message because you are subscribed to a topic in the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/rubyonrails-core/TqPPisZ3ttU/unsubscribe.
To unsubscribe from this group and all its topics, send an email to rubyonrails-co...@googlegroups.com.

[Isaac Betesh]

3) rails-api (https://github.com/rails/rails/blob/master/actionpack/lib/action_controller/api.rb) is merged into master (though not into 4.2-stable).  "ApplicationController < ActionController::API" would would be a very reasonable use case for having no need for capybara.  As this is going to part of Rails in the next major release, it seems that Rails applications that only respond_to JSON will become more common.  While it's still possible to use capybara for such applications, there are much more lightweight options (such as Webrick + Net::HTTP, which I often use instead of capybara for lightweight sinatra apps).  So even if currently most Rails apps use capybara, a growing number of them won't with the next release.

[iybe...@gmail.com]

Rafael, 

I made 3 strong arguments in response to your point about capybara.  I would have appreciated a response explaining if any of my reasoning is wrong here.

This is not a difficult PR and I still feel the case for it is strong but I don't want to start working on it while I'm waiting to hear from you.

[Rafael Mendonça França]

I think it is worth to experiment yes, I don't want to decline anything without seeing the code and how it would feel. I can see advantages of doing that.

[Isaac Betesh]

Thank you.

I will aim for submitting a PR over the coming weekend.

--
You received this message because you are subscribed to a topic in the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/rubyonrails-core/TqPPisZ3ttU/unsubscribe.
To unsubscribe from this group and all its topics, send an email to rubyonrails-co...@googlegroups.com.