[Rails-core] Re: Error pages are really just static assets. Why not make them part of the asset pipeline?
I can't speak for others, but for me it meant that I had no interest in the idea. The suggestions in that PR of putting non-fingerprinted files in `public/assets` reveals a misunderstanding of how you're meant to configure that directory for long cache expiry (and hence, a misunderstanding of the entire point of the asset pipeline and fingerprinting files). Your suggestion is at least feasible, although I think (and in my experience) the use case you explain is much more easily resolved by adding some ERB processing into your deploy scripts. It's basically (as far as I can tell) your same idea, but just done in your own rake task as part of your deploy instead of added to the existing precompile task. On Wednesday, August 5, 2015 at 7:49:43 AM UTC-6, iybe...@gmail.com wrote: What does it mean when nobody replies after almost a week? Does that mean everyone thinks it's a good idea or everyone thinks it's a bad idea? Should I just take my chances and submit a PR? On Friday, July 31, 2015 at 10:05:44 AM UTC-4, iybe...@gmail.com wrote: https://github.com/rails/sprockets-rails/issues/49 (which is locked so no further discussion is possible there) debates whether asset precompile should create a non-digest version of the assets. There are a few valid use cases for this, and I do not have solutions for all of them, but I just thought of a solution to one--static error pages. @pelargir, in some of the first comments on the issue ( https://github.com/rails/sprockets-rails/issues/49#issuecomment-20529994, https://github.com/rails/sprockets-rails/issues/49#issuecomment-20531267), stated the issue: Removing generation of non-digest files also makes is impossible to have static 404 error pages that use the same images, JS, and CSS as the remainder of the site (without making duplicate non-digest copies of those files in /public). But then I end up with two copies of the same file in my app. Am I supposed to copy the precompiled file into /public and remove the fingerprint from the filename? So then, if I change the file in the future, I have to precompile, copy, and rename all over again? My idea is to make the static error pages part of the asset pipeline. Instead of generating public/500.html, public/404.html, etc, a new rails app should generate app/assets/html/500.en.html.erb, app/assets/html/404.en.html.erb, etc. app/assets/html/* should by default be included in config.assets.precompile (which I believe it is anyway since it's in app/assets and the extension is neither .css or .js). There are 2 places where the behavior would be significantly different than of all other assets, so things would get a little tricky here: 1) Unlike other assets, these would need to be precompiled in the context of a layout. This could be either the same application layout as the rest of the application (app/views/layouts/application.html.erb ), or the rails new generator could create a separate app/assets/html/error_layout.en.html.erb (which itself should be excluded from precompiled). Telling sprockets when to compile in the context of a layout could be tricky, but an alternative is just to code that into the view. Below is an example from an existing project of mine. It's in HAML, not ERB, but still illustrates the point: # app/assets/html/500.en.html - layout = Rails.root.join(app, assets, html, error_layout.en.haml) = Haml::Engine.new(File.read layout).render do %h5 We're sorry, but something went wrong. %p Our developers are working on it. 2) Error pages currently live in public, but with this approach, they would end up in public/assets, and would have digests in their file names. So we would need to change https://github.com/rails/rails/blob/v4.2.3/actionpack/lib/action_dispatch/middleware/public_exceptions.rb#L44 to look in public/assets and use the appropriate digests, and add a Depracation warning (that shows up in development environment) if the error pages are found in public. -- 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-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.
[Rails-core] Re: Nokogiri adds a lot of weight to ActionView
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. On Monday, July 13, 2015 at 6:37:28 PM UTC-4, iybe...@gmail.com wrote: 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 https://github.com/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. -- 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-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.
Re: [Rails-core] Re: Nokogiri adds a lot of weight to ActionView
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. On Mon, Aug 10, 2015 at 2:11 PM iybet...@gmail.com wrote: 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. On Monday, July 13, 2015 at 6:37:28 PM UTC-4, iybe...@gmail.com wrote: 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 https://github.com/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. -- 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-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout. -- 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-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.
Re: [Rails-core] Re: Nokogiri adds a lot of weight to ActionView
Thank you. I will aim for submitting a PR over the coming weekend. On Mon, Aug 10, 2015 at 1:18 PM, Rafael Mendonça França rafaelmfra...@gmail.com wrote: 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. On Mon, Aug 10, 2015 at 2:11 PM iybet...@gmail.com wrote: 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. On Monday, July 13, 2015 at 6:37:28 PM UTC-4, iybe...@gmail.com wrote: 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 https://github.com/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. -- 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-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout. -- 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-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout. -- 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-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For
