Fix images precompilation for sprockets 4 #787

estepnv · 2017-03-22T11:02:06Z

No description provided.

houndci-bot · 2017-03-22T11:02:17Z

lib/administrate/engine.rb

+    # regex no longer supported by assets.precompile
+    # sprockets-rails 3 tracks down the calls to `font_path` and `image_path`
+    # and automatically precompiles the referenced assets.
+    unless Sprockets::Rails::VERSION.starts_with?('3')


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-03-22T11:02:17Z

lib/administrate/engine.rb

@@ -24,7 +24,13 @@ class Engine < ::Rails::Engine
    @@javascripts = []
    @@stylesheets = []

-    Engine.config.assets.precompile << /\.(?:svg)\z/
+


Extra blank line detected.

houndci-bot · 2017-03-22T11:38:48Z

lib/administrate/engine.rb

@@ -24,7 +24,11 @@ class Engine < ::Rails::Engine
    @@javascripts = []
    @@stylesheets = []

-    Engine.config.assets.precompile << /\.(?:svg)\z/
+    if Sprockets::Rails::VERSION.start_with?("4")
+      Dir["../../app/assets/images/**/*.svg"].each { |image| Engine.config.assets.precompile << image }


Line is too long. [103/80]

houndci-bot · 2017-03-22T12:13:22Z

lib/administrate/engine.rb

@@ -24,7 +24,13 @@ class Engine < ::Rails::Engine
    @@javascripts = []
    @@stylesheets = []

-    Engine.config.assets.precompile << /\.(?:svg)\z/
+    if Sprockets::VERSION.start_with?("4")
+      Dir[Engine.root.join('app/assets/images/**/*.svg')].each do |image|


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

BenMorganIO · 2017-03-23T19:26:03Z

@estepan I was going to make a PR for this myself, but it looks like you beat me to it!

BenMorganIO · 2017-03-23T19:27:22Z

lib/administrate/engine.rb

-    Engine.config.assets.precompile << /\.(?:svg)\z/
+    if Sprockets::VERSION.start_with?("4")
+      Dir[Engine.root.join("app/assets/images/**/*.svg")].each do |image|
+        Engine.config.assets.precompile << File.expand_path(image)


I think we should have an assets initializer and whitelist the SVG assets, but this would work as well :)

BenMorganIO · 2017-03-23T19:27:55Z

lib/administrate/engine.rb

+        Engine.config.assets.precompile << File.expand_path(image)
+      end
+    else
+      Engine.config.assets.precompile << /\.(?:svg)\z/


Your new method would work for Sprockets 3, what do you think about removing this line here entirely?

btw you are right) I was in hurry writing this stuff) thanks)

BenMorganIO · 2017-03-23T19:30:07Z

@estepan what do you think of the implementation in https://github.com/thoughtbot/administrate/pull/648/files#diff-65032410b52e54dc1cfa4faaced808d3R27?

…images-precompilation-sprockets4

houndci-bot · 2017-03-27T15:20:46Z

lib/administrate/engine.rb

+        Engine.config.assets.precompile << File.expand_path(image)
+      end
+    else
+      Engine.config.assets.precompile += %w( administrate/manifest.js )


Do not use spaces inside percent literal delimiters.

houndci-bot · 2017-03-27T15:20:46Z

lib/administrate/engine.rb

@@ -24,7 +24,13 @@ class Engine < ::Rails::Engine
    @@javascripts = []
    @@stylesheets = []

-    Engine.config.assets.precompile << /\.(?:svg)\z/
+    if Sprockets::VERSION.start_with?('2')


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

estepnv · 2017-03-27T15:25:16Z

@BenMorganIO I've pulled latest master. But build fails for sprockets-4 appraisal

Failure/Error: <%= stylesheet_link_tag css_path %>
     
     ActionView::Template::Error:
       couldn't find file '/Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/bourbon-5.0.0.beta.7/core/bourbon/utilities/_directional-property.scss'
       Checked in these paths: 
         /Users/estepnv/Projects/administrate/spec/example_app/app/assets/config
         /Users/estepnv/Projects/administrate/spec/example_app/app/assets/images
         /Users/estepnv/Projects/administrate/spec/example_app/app/assets/javascripts
         /Users/estepnv/Projects/administrate/spec/example_app/app/assets/stylesheets
         /Users/estepnv/Projects/administrate/spec/example_app/vendor/assets/javascripts
         /Users/estepnv/Projects/administrate/spec/example_app/vendor/assets/stylesheets
         /Users/estepnv/Projects/administrate/app/assets/config
         /Users/estepnv/Projects/administrate/app/assets/images
         /Users/estepnv/Projects/administrate/app/assets/javascripts
         /Users/estepnv/Projects/administrate/app/assets/stylesheets
         /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/selectize-rails-0.12.4/vendor/assets/javascripts
         /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/selectize-rails-0.12.4/vendor/assets/stylesheets
         /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/normalize-rails-4.1.1/vendor/assets/stylesheets
         /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/neat-1.8.0/app/assets/stylesheets
         /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/momentjs-rails-2.17.1/vendor/assets/javascripts
         /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/jquery-rails-4.3.1/vendor/assets/javascripts
         /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/datetime_picker_rails-0.0.7/app/assets/fonts
         /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/datetime_picker_rails-0.0.7/app/assets/javascripts
         /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/datetime_picker_rails-0.0.7/app/assets/stylesheets
     # ./app/views/administrate/application/_stylesheet.html.erb:11:in `block in ___sers_estepnv__rojects_administrate_app_views_administrate_application__stylesheet_html_erb__2142318194235325576_70107045959900'
     # ./app/views/administrate/application/_stylesheet.html.erb:10:in `each'
     # ./app/views/administrate/application/_stylesheet.html.erb:10:in `___sers_estepnv__rojects_administrate_app_views_administrate_application__stylesheet_html_erb__2142318194235325576_70107045959900'
     # ./app/views/layouts/administrate/application.html.erb:22:in `___sers_estepnv__rojects_administrate_app_views_layouts_administrate_application_html_erb___1142917166974433817_70107011718600'
     # ./app/controllers/administrate/application_controller.rb:33:in `edit'
     # ./spec/features/edit_page_spec.rb:7:in `block (2 levels) in <top (required)>'
     # ./spec/support/background_jobs.rb:14:in `block (3 levels) in <top (required)>'
     # ./spec/support/background_jobs.rb:5:in `run_background_jobs_immediately'
     # ./spec/support/background_jobs.rb:13:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # Sprockets::FileNotFound:
     #   couldn't find file '/Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/bourbon-5.0.0.beta.7/core/bourbon/utilities/_directional-property.scss'
     #   Checked in these paths: 
     #     /Users/estepnv/Projects/administrate/spec/example_app/app/assets/config
     #     /Users/estepnv/Projects/administrate/spec/example_app/app/assets/images
     #     /Users/estepnv/Projects/administrate/spec/example_app/app/assets/javascripts
     #     /Users/estepnv/Projects/administrate/spec/example_app/app/assets/stylesheets
     #     /Users/estepnv/Projects/administrate/spec/example_app/vendor/assets/javascripts
     #     /Users/estepnv/Projects/administrate/spec/example_app/vendor/assets/stylesheets
     #     /Users/estepnv/Projects/administrate/app/assets/config
     #     /Users/estepnv/Projects/administrate/app/assets/images
     #     /Users/estepnv/Projects/administrate/app/assets/javascripts
     #     /Users/estepnv/Projects/administrate/app/assets/stylesheets
     #     /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/selectize-rails-0.12.4/vendor/assets/javascripts
     #     /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/selectize-rails-0.12.4/vendor/assets/stylesheets
     #     /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/normalize-rails-4.1.1/vendor/assets/stylesheets
     #     /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/neat-1.8.0/app/assets/stylesheets
     #     /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/momentjs-rails-2.17.1/vendor/assets/javascripts
     #     /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/jquery-rails-4.3.1/vendor/assets/javascripts
     #     /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/datetime_picker_rails-0.0.7/app/assets/fonts
     #     /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/datetime_picker_rails-0.0.7/app/assets/javascripts
     #     /Users/estepnv/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/datetime_picker_rails-0.0.7/app/assets/stylesheets
     #   ./app/views/administrate/application/_stylesheet.html.erb:11:in `block in ___sers_estepnv__rojects_administrate_app_views_administrate_application__stylesheet_html_erb__2142318194235325576_70107045959900'

I spent half an hour on it but failed.

https://github.com/thoughtbot/administrate/pull/648/files#diff-e927995a5a507ceba19f77e1416e130cR6 bourbon 4.2.7 is used so everything is ok.

Any ideas?

BenMorganIO · 2017-04-04T19:30:03Z

Just got #814 merged! Your PR does more than just fix the assets, it adds a manifest to the application. Also, I don't think we need to have an Appraisal just for Sprockets. I'd love to stay out of that complexity.

If you'd like Administrate to use a manifest, could you open a simple PR just for that?

houndci-bot reviewed Mar 22, 2017

View reviewed changes

estepnv changed the title ~~Fix images precompilation sprockets 4~~ Fix images precompilation for sprockets 4 Mar 22, 2017

estepnv force-pushed the fix-images-precompilation-sprockets4 branch 3 times, most recently from 0cfc562 to 396182b Compare March 22, 2017 11:38

houndci-bot reviewed Mar 22, 2017

View reviewed changes

estepnv force-pushed the fix-images-precompilation-sprockets4 branch 6 times, most recently from b1cea65 to b977a19 Compare March 22, 2017 12:13

houndci-bot reviewed Mar 22, 2017

View reviewed changes

Add sprockets 4 compatibility

c5aa4d4

estepnv force-pushed the fix-images-precompilation-sprockets4 branch from b977a19 to c5aa4d4 Compare March 22, 2017 12:16

BenMorganIO reviewed Mar 23, 2017

View reviewed changes

BenMorganIO assigned BenMorganIO and unassigned BenMorganIO Mar 23, 2017

Merge branch 'master' of github.com:thoughtbot/administrate into fix-…

ea9ca89

…images-precompilation-sprockets4

houndci-bot reviewed Mar 27, 2017

View reviewed changes

Add sprockets-4 appraisal

db996e8

estepnv force-pushed the fix-images-precompilation-sprockets4 branch from a6eb13b to db996e8 Compare March 27, 2017 15:26

BenMorganIO closed this Apr 4, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix images precompilation for sprockets 4 #787

Fix images precompilation for sprockets 4 #787

estepnv commented Mar 22, 2017

houndci-bot Mar 22, 2017

houndci-bot Mar 22, 2017

houndci-bot Mar 22, 2017

houndci-bot Mar 22, 2017

BenMorganIO commented Mar 23, 2017

BenMorganIO Mar 23, 2017

BenMorganIO Mar 23, 2017

estepnv Mar 27, 2017

BenMorganIO commented Mar 23, 2017

houndci-bot Mar 27, 2017

houndci-bot Mar 27, 2017

estepnv commented Mar 27, 2017

BenMorganIO commented Apr 4, 2017

Fix images precompilation for sprockets 4 #787

Fix images precompilation for sprockets 4 #787

Conversation

estepnv commented Mar 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenMorganIO commented Mar 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenMorganIO commented Mar 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estepnv commented Mar 27, 2017

BenMorganIO commented Apr 4, 2017