rails / mission_control-jobs

Dashboard and Active Job extensions to operate and troubleshoot background jobs
MIT License
611 stars 71 forks source link

Compatibility with `sprockets` #169

Closed jhovad closed 1 month ago

jhovad commented 1 month ago

Hi! I noticed that PR#87 introduced a dependency on propshaft.

Was this change intentional, and is it necessary?

It seems that this update has made the gem incompatible with projects that are still using sprockets.

We’d really love to continue using this gem with sprockets if possible. Thank you!

ryantaylor commented 1 month ago

This is the commit that added the dependency: https://github.com/rails/mission_control-jobs/commit/d8bee68a872b9b2742bcc4fb60b4550282983594, looks like it was made back in July on a PR that was first opened in February: https://github.com/rails/mission_control-jobs/pull/87

Seems the incompatibility was unexpected, I agree that it would be nice to be able to continue using this tool with sprockets (I am also not able to use the latest version because of this).

aesmail commented 1 month ago

Would like to add in that upgrading from 0.3.1 to 0.3.2 broke our build because of the introduction of propshaft as a dependency. Sticking to 0.3.1 for now. We wouldn't mind moving to propshaft and that is the plan, but the change was very unexpected.

onerinas commented 1 month ago

Can confirm, build started failing now with 0.3.2 in a project using sprockets. I'll revert to 0.3.1 as well.

Relevant part of the log incase it helps:

NoMethodError: undefined method `cached' for an instance of Propshaft::Assembly (NoMethodError)
16:26:56
@cached ||= environment.cached if environment
16:26:56
^^^^^^^
16:26:56
/var/deploy/hifiveweb/web_head/shared/bundle/ruby/3.3.0/gems/sprockets-4.2.1/lib/rake/sprocketstask.rb:43:in `cached'
16:26:56
/var/deploy/hifiveweb/web_head/shared/bundle/ruby/3.3.0/gems/sprockets-rails-3.5.2/lib/sprockets/rails/task.rb:45:in `manifest'
16:26:56
/var/deploy/hifiveweb/web_head/shared/bundle/ruby/3.3.0/gems/sprockets-4.2.1/lib/rake/sprocketstask.rb:148:in `with_logger'
16:26:56
/var/deploy/hifiveweb/web_head/shared/bundle/ruby/3.3.0/gems/sprockets-rails-3.5.2/lib/sprockets/rails/task.rb:66:in `block (2 levels) in define'
16:26:56
/var/deploy/hifiveweb/web_head/shared/bundle/ruby/3.3.0/gems/rake-13.2.1/exe/rake:27:in `'
16:26:56
/usr/local/bin/ruby_executable_hooks:22:in `eval'
16:26:56
/usr/local/bin/ruby_executable_hooks:22:in `'
16:26:56
Tasks: TOP => assets:precompile
patricklewis commented 1 month ago

This move to Propshaft has resulted in Sprockets-based test suite failures for me when upgrading from 0.3.1 to 0.3.2:

     ActionView::Template::Error:
       Asset `mission_control/jobs/application.js` was not declared to be precompiled in production.
       Declare links to your assets in `app/assets/config/manifest.js`.

         //= link mission_control/jobs/application.js

       and restart your server
     # ./app/views/layouts/application.html.erb:18:in `_app_views_layouts_application_html_erb___4153382308342819861_75060'
     # ./spec/requests/authentication_spec.rb:46:in `block (2 levels) in <main>'
     # ------------------
     # --- Caused by: ---
     # Sprockets::Rails::Helper::AssetNotPrecompiledError:
     #   Asset `mission_control/jobs/application.js` was not declared to be precompiled in production.
     #   Declare links to your assets in `app/assets/config/manifest.js`.
     #   
     #     //= link mission_control/jobs/application.js
     #   
     #   and restart your server
     #   ./app/views/layouts/application.html.erb:18:in `_app_views_layouts_application_html_erb___4153382308342819861_75060'
wbotelhos commented 1 month ago

My deploy broken using the version 0.3.2. I'm surprised because it should be a bug fix bump version, but we have much more stuff going on.

#15 6.322 bin/rails aborted!
#15 6.325 TypeError: no implicit conversion of Propshaft::Assembly into String (TypeError)
#15 6.325 
#15 6.325       @directory = File.expand_path(@directory) if @directory
#15 6.325                                     ^^^^^^^^^^
#15 6.325 /app/vendor/bundle/ruby/3.3.0/gems/sprockets-4.2.1/lib/sprockets/manifest.rb:43:in `expand_path'
#15 6.325 /app/vendor/bundle/ruby/3.3.0/gems/sprockets-4.2.1/lib/sprockets/manifest.rb:43:in `initialize'
#15 6.325 /app/vendor/bundle/ruby/3.3.0/gems/sprockets-rails-3.5.2/lib/sprockets/railtie.rb:216:in `new'
#15 6.325 /app/vendor/bundle/ruby/3.3.0/gems/sprockets-rails-3.5.2/lib/sprockets/railtie.rb:216:in `build_manifest'

The new version made me import some assets in the manifest.js:

//= link mission_control/jobs/application.css
//= link mission_control/jobs/application.js
//= link mission_control/jobs/controllers/application.js
//= link mission_control/jobs/controllers/form_controller.js
//= link mission_control/jobs/controllers/index.js
//= link mission_control/jobs/helpers/debounce_helpers.js
//= link mission_control/jobs/helpers/index.js

Would be nice a clean support for Sprocket since I believe the most of people didn't migrated to Propshaft and may be using ESBuild or similar, like me.

rosa commented 1 month ago

Hey! I just released version 0.3.3, reverting the change that caused this https://github.com/rails/mission_control-jobs/releases/tag/v0.3.3.

FWIW, until this is more stable, you can expect breaking changes with little ceremony, but I admit I didn't correctly assess the breakage this one change would cause. Sorry about that!