jaynetics / activeadmin_assets

Run ActiveAdmin v4 without asset setup
MIT License
4 stars 0 forks source link

Possible Enhancements #2

Open henrikbjorn opened 1 month ago

henrikbjorn commented 1 month ago

Maybe it would be possible to inject an instance of ActionDispatch::Static to serve the static files instead of a custom middleware and regex matching :)

henrikbjorn commented 1 month ago

Another thought

If the compiled css and js are in app/assets then they would be picked up by Propshaft or Sprockets by themselves and then there is no need for the middleware at all. Since a normal stylesheet_link_tag would find them.

jaynetics commented 1 month ago

hey, thanks for the feedback!

Maybe it would be possible to inject an instance of ActionDispatch::Static to serve the static files instead of a custom middleware and regex matching :)

it was the first thing i tried and there were some conflicts with an existing Static middleware, but i guess it can be done.

one downside of Static is that it is not that fast, as opposed to regexps which are quite fast in ruby. my use case is running activeadmin in a rails app in API mode, and that app should be able to serve a lot of requests, so one goal here was to not slow down the middleware stack:

require 'benchmark/ips'
require 'action_dispatch'
require 'activeadmin_assets'

app = ->(_) {}
env = { "PATH_INFO" => "/index.json", "REQUEST_METHOD" => "GET" }
m1 = ActiveAdminAssets::Middleware.new(app)
m2 = ActionDispatch::Static.new(app, 'some_path')

Benchmark.ips do |x|
  x.report('ActiveAdminAssets::Middleware') { m1.call(env) }
  x.report('ActionDispatch::Static') { m2.call(env) }
end

# Comparison:
# ActiveAdminAssets::Middleware:  4072263.3 i/s
# ActionDispatch::Static:   155206.9 i/s - 26.24x  slower

if Static does something relevant in a better way i'm certainly happy to reconsider, though :)

If the compiled css and js are in app/assets then they would be picked up by Propshaft or Sprockets by themselves and then there is no need for the middleware at all. Since a normal stylesheet_link_tag would find them.

can you elaborate?

if you already have propshaft or sprockets and an assets:precompile step, i'm not sure this gem is all that useful.

or are you suggesting this gem should become a full Rails::Engine that itself uses sprockets or propshaft? then people might need to mount it, and i think we'd have to make sure that it injects its assets directory into the beginning of whatever lookup list of directories stylesheet_link_tag uses in the main app (?)

henrikbjorn commented 1 month ago

At work we have a gem for our design system which is based on Tailwind. In this case we pre-optimizing our icon set. But previously we did a precompile .css file also.

What we did what dump that in app/assets/gem_name/stylesheet and add the as a precompiled step.

Which allows us to reference that .css file with a standard stylesheet_link_tag 'my_compiled_css.css'.

For images they are in app/assets/images/gem_name and can be used with inline_svg('gem_name/some.svg').

I think that would allow us to just use Propshaft and skip middleware

jaynetics commented 1 month ago

It would be nice to make this gem even slimmer, but another goal is that it should "just work" in any given app. I'm not sure that would still be the case with this approach. It might depend on whether sprockets or propshaft are present in the main app (as both override stylesheet_link_tag and methods called by it), whether action_controller.asset_host is set, whether relative_url_root is set, and possibly other things...

henrikbjorn commented 1 month ago

If you look at turbo-rails and stimulus-rails that is what they are doing basically.

I think that in 99% of cases Propshaft will be present and thus the helpers will be available.

If using ViteRuby then you can do a simple override over the partial _html_head.html.erb and use their helpers.

The problem currently, is that I use ViteRuby and "cant" really use the gem as I don't want another middleware in my stack when it could be a simple Hybrid and allow me to still use Vite

jaynetics commented 1 month ago

it could be a simple Hybrid and allow me to still use Vite

are you saying activeadmin_assets interferes in any way? this would be strange because it only patches asset URLs within activeadmin views, and even there only for assets with specific names.

If you look at turbo-rails and stimulus-rails that is what they are doing basically.

yeah, and they don't always just work out of the box, e.g. there are cases were their assets need to be manually excluded from precompilation.

I think that in 99% of cases Propshaft will be present and thus the helpers will be available.

sprockets is still the default and will remain so until rails 8, so i guess the reality of maintained rails apps is closer to something roughly like this:

70% have sprockets 10% have propshaft 20% have neither (rails new --api)

and of course not everyone who has either gem runs assets:precompile. the more i think about it, the more permutations there are, and the more hesitant i am to rely on the asset pipeline 😁

another option could be adding a CDN mode to activeadmin_assets:

i wouldn't want to make that the default due to its security and privacy implications, but it would be another way to opt out of the middleware.

henrikbjorn commented 1 month ago

I think we might be trying to solve the same thing differently.

In my view the pain with ActiveAdmin 4 is the need to install Tailwind and do the build through that.

Solving that in my view would be to build the default .css file and include it into app/assets something and then allow Sprockets / Propshaft to do its thing.

With Vite that would also work great since I can just reference the asset file from inside my Vite setup https://henrikbjorn.medium.com/til-how-to-use-3rd-party-rubygem-assets-with-vite-ruby-rails-145b8b7d663c

To me that would be the best of both worlds.

Having a optional static middleware can still be present.


I can get the current setup to work if the assets would also be included as non .gz assets.

Then in Vite i can just @import "gems/activeadmin_assets/lib/activeadmin_assets/assets/activeadmin.css (and the same with the .js file)

In my Gemfile I just do gem "activeadmin_assets", require: false and then the middleware isn't activated.

jaynetics commented 1 month ago

I can get the current setup to work if the assets would also be included as non .gz assets.

makes sense! i've created a branch to include the uncompressed CSS, you can give it a try with

gem 'activeadmin_assets', git: 'https://github.com/jaynetics/activeadmin_assets.git', branch: 'include-uncompressed-css'

(and the same with the .js file)

oh, i'm not bundling the JS into a single file, i'm just including gzipped copies of each of the 10 files from activeadmin. but if you have importmap and sprockets/propshaft, activeadmin adds its JS to assets.paths itself, so precompilation should pick it up, or you might be able to import the JS from the activeadmin gem itself?