-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Sprockets v4 to the DummyApp #3379
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I think we should leave the file blank on purpose. Actually this file should be only necessary if people actually set Rails.application.assets.precompile += ['manifest.js']
, but this is something to discuss with the Rails team.
core/lib/spree/testing_support/dummy_app/assets/config/manifest.js
Outdated
Show resolved
Hide resolved
@@ -87,6 +87,7 @@ class Application < ::Rails::Application | |||
config.action_controller.include_all_helpers = false | |||
|
|||
if config.respond_to?(:assets) | |||
config.assets.config_manifest = File.expand_path('dummy_app/assets/config/manifest.js', __dir__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love your PR 👍
9789abc
to
cc5a4f5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great!
cc5a4f5
to
2fbe9c3
Compare
2fbe9c3
to
c719179
Compare
8163a5f
to
4024df7
Compare
This is needed from Sprockets, since v4. It contains all dependencies that needs to be compiled. Co-Authored-By: Elia Schito <[email protected]>
4024df7
to
1a35ea5
Compare
Removing the dependency on sprockets < 4 rack is now unbounded, but rack 3 is only supported from Rails 7.1 up.
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
[v4.3] Add support for Sprockets v4 to the DummyApp (backports #3379)
🛑 BLOCKED BY rails/sprockets-rails#446Description
Sprockets 4 introduced a mandatory configuration manifest at a specific location. We don't have the ability to set it at that specific location in the DummyApp we use for our specs.
I've opened rails/sprockets-rails#446 on sprockets-rails which should fix this issue. I'll leave this PR open (working with that branch) waiting to know if that PR will be accepted.Update: Now an empty manifest will be written in the expected location inside
spec/dummy
during the dummy app setup.Closes #3374, closes #3376
After merging this we need to revertThis also reverts #3378.Checklist: