Fix hyrax js not rendering correctly #5672
Conversation
|
|
||
| //= require hyrax | ||
| JS | ||
| insert_into_file 'app/assets/javascripts/application.js', after: "//= require blacklight/blacklight\n" do |
There was a problem hiding this comment.
With the generation of a new app using Rails 6, the application.js generated is missing //= require_tree ., which led to the failure of the inseration of the Hyrax js. This change insures the inclusion of hyrax is inserted below blacklight/blacklight, which is a core dependency that will always be included when the test app is generated.
| @@ -5,6 +5,14 @@ class TestAppGenerator < Rails::Generators::Base | |||
| # so the following path gets us to /path/to/hyrax/spec/test_app_templates/ | |||
| source_root File.expand_path('../../../../spec/test_app_templates/', __FILE__) | |||
|
|
|||
| def install_turbolinks | |||
| gem 'turbolinks', '~> 5' | |||
There was a problem hiding this comment.
The test app generated using Rails 6 and engine_cart is missing turbolinks by default, and the test app or any app dependent on Hyrax cannot function properly unless it includes turbolinks. I tried including turbolinks in the hyrax gemspec, but that does not set it up correctly, and leads to the following error:
couldn't find file 'turbolinks' with type 'application/javascript'
| @@ -198,7 +206,7 @@ def configure_action_cable_to_use_redis | |||
| end | |||
|
|
|||
| def install_universal_viewer | |||
| raise '`yarn install` failed!' unless system('./bin/yarn install') | |||
| raise '`yarn install` failed!' unless system('yarn install') | |||
There was a problem hiding this comment.
./bin/yarn install fails while yarn install succeeds, which is why I made the switch. Unless there is a particular reason behind the use of ./bin/yarn install, this change should be ok.
tasks/hyrax_dev.rake
Outdated
| '--skip-bootsnap', | ||
| '--skip-listen', | ||
| '--skip-test', | ||
| '--skip-javascript', |
There was a problem hiding this comment.
I added an override of the engine_cart:create_test_rails_app primarily to enforce skipping javascript in the generation of a new Rails app by default. In Rails 6, unless you use --skip-javascript, the webpacker gem is installed and leads to a variety of changes and errors on the app.
There was a problem hiding this comment.
Instead of overriding engine_cart, can we instead update relevant documentation and the CI config to insert --skip-javascript using the ENGINE_CART_RAILS_OPTIONS environment variable?
There was a problem hiding this comment.
@dlpierce I added the change you recommended, and I deleted the engine_cart.yml file, because it overrides the options set in the circleci config, and currently has a bug that does not allow for adding multiple options. Please refer to this thread for more details: cbeer/engine_cart#102
abelemlih
force-pushed
the
abel-hyrax_js_fix
branch
from
14463b3 to
3af711d
Compare
tasks/hyrax_dev.rake
Outdated
| '--skip-bootsnap', | ||
| '--skip-listen', | ||
| '--skip-test', | ||
| '--skip-javascript', |
There was a problem hiding this comment.
Instead of overriding engine_cart, can we instead update relevant documentation and the CI config to insert --skip-javascript using the ENGINE_CART_RAILS_OPTIONS environment variable?
abelemlih
force-pushed
the
abel-hyrax_js_fix
branch
from
9643936 to
df201e3
Compare
I attached notes to each to clarify the reasoning behind it and what it is intended to achieve.