search

railsadminteam / rails_admin

RailsAdmin is a Rails engine that provides an easy-to-use interface for managing your data
MIT License
7.87k stars 2.25k forks source link

ActionText detection in app/assets/javascripts/rails_admin/application.js.erb produces false positives #3659

Closed jdufresne closed 6 months ago

jdufresne commented 7 months ago

Describe the bug

In app/assets/javascripts/rails_admin/application.js.erb, there is detection for the use of ActionText:

https://github.com/railsadminteam/rails_admin/blob/13c90fdd95c3755035b44264dc61e4b65c09ff12/app/assets/javascripts/rails_admin/application.js.erb#L27-L30

This checks if the ActionText module exists by calling defined?.

My application does not use ActionText, but the ActionText module exists for other reasons. For me, this gets loaded by bootstrap_form gem, which uses the ERB helpers only:

https://github.com/bootstrap-ruby/bootstrap_form/blob/ed3eda885c2145a364e8affd759f42b03ff6ee7c/lib/bootstrap_form.rb#L1-L9

When Rails bootstrap_form loads the global module ActionText for its helpers, it "tricks" railsadmin into believing ActionText is in use, which causes and issue when compiling assets: 

$ bundle exec rails assets:precompile
bin/rails aborted!
Sprockets::FileNotFound: couldn't find file 'trix' with type 'application/javascript' (Sprockets::FileNotFound)
Checked in these paths:
  .../app/assets/config
  .../app/assets/images
  .../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/app/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/app/assets/stylesheets
  .../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/vendor/assets/fonts
  .../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/vendor/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/vendor/assets/stylesheets
  .../vendor/bundle/ruby/3.2.0/gems/turbo-rails-1.5.0/app/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/nested_form-0.3.2/vendor/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/bootstrap_form-5.4.0/app/assets/stylesheets
  .../vendor/bundle/ruby/3.2.0/gems/actioncable-7.1.2/app/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/activestorage-7.1.2/app/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/actionview-7.1.2/app/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/actionview-7.1.2/lib/assets/compiled
  .../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/src
.../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/app/assets/javascripts/rails_admin/application.js.erb:24

Reproduction steps

Expected behavior

Assets are compiled and the ActionText dependencies are skipped.

Is there a more robust approach to detecting ActionText was loaded?

Additional context

$ bundle exec rails assets:precompile
bin/rails aborted!
Sprockets::FileNotFound: couldn't find file 'trix' with type 'application/javascript' (Sprockets::FileNotFound)
Checked in these paths:
  .../app/assets/config
  .../app/assets/images
  .../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/app/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/app/assets/stylesheets
  .../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/vendor/assets/fonts
  .../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/vendor/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/vendor/assets/stylesheets
  .../vendor/bundle/ruby/3.2.0/gems/turbo-rails-1.5.0/app/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/nested_form-0.3.2/vendor/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/bootstrap_form-5.4.0/app/assets/stylesheets
  .../vendor/bundle/ruby/3.2.0/gems/actioncable-7.1.2/app/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/activestorage-7.1.2/app/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/actionview-7.1.2/app/assets/javascripts
  .../vendor/bundle/ruby/3.2.0/gems/actionview-7.1.2/lib/assets/compiled
  .../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/src
.../vendor/bundle/ruby/3.2.0/gems/rails_admin-3.1.2/app/assets/javascripts/rails_admin/application.js.erb:24
mshibuya commented 7 months ago

Just remove what you don't need. I suppose you're already accustomed to modify auto-generated codes, like ones generated by rails new, right?

jdufresne commented 7 months ago

Hi @mshibuya, what auto-generated code are you referring to in your comment? When you say "Just remove what you don't need.", what are you suggesting I remove?

This issue is coming from the interactions of two gems outside my control -- one being railsadmin due to its sub-optimal feature detection of if defined?(ActionText).

If you still believe the issue lies within my own project, can you provide more detail? I'm certainly happy to make adjustments there.

mshibuya commented 7 months ago

Ah sorry my bad, please ignore above. I mistook it as a template erb for the generator.

Then I agree that having something better is desirable. I'll see if it is possible soon.

jdufresne commented 7 months ago

Here is an example application that demonstrates the error:

https://github.com/jdufresne/rails-example-action-text-issue

To experience the error, run:

bundle install
bundle exec rails assets:precompile

Deleting out RailAdmin's <% if defined?(ActionText) && Rails.gem_version >= Gem::Version.new('7.0') %> makes the error go away.

wvengen commented 7 months ago

I'm trying to think of a use-case where one would load ActionText and not want to precompile the assets. Perhaps when it is used in a frontend part, but there is a desire to keep it disabled in the backend (Rails Admin). In this case, the asset compilation of ActionText would not be necessary. It brings a little more load for the backend part, but in most cases I think that would be preferable to making it more difficult to actually use ActionText. So I think what Rails Admin has now is a great default.

One possible solution would be to provide a global Rails Admin config option to actively disable ActionText.

Nevertheless, eagerly requiring ActionText in the gem you're referring to is something I would discourage - I would find it quite unexpected. If I want to load it, I'd put it in the Gemfile, where I can even add require: false if I want it available but not loaded. There could be some very confusing load order issues here (what if a gem that enables/disables functionality based on the availability of ActionText is loaded before ActionText is loaded), that would speak for eagerly loading and making it explicit whether you'd want to use ActionText or not.

I'm curious if someone has an idea on how to handle optional dependencies like this cleanly.

jdufresne commented 7 months ago

Nevertheless, eagerly requiring ActionText in the gem you're referring to is something I would discourage - I would find it quite unexpected.

I agree and I've opened an issue with that project at: https://github.com/bootstrap-ruby/bootstrap_form/issues/719

I have also opened this issue in RailsAdmin to see if there is a more robust way to detect an enabled engine than defined?(ActionText).