-
-
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
Remove use of require_tree in admin javascripts #1613
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.
This is great. Thanks for doing the actual hard work, John!
One small thing left, though
//= require spree/backend/address_states | ||
//= require spree/backend/adjustments | ||
//= require spree/backend/admin | ||
//= require spree/backend/backbone-overrides |
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.
You already required this in L15
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.
Fixed
8c870eb
to
6a7d363
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.
I think in the future we could handle all folders like the spree/backend/templates
folder, but this is great for now and fixes #1567
Using this fix in my demo app https://github.com/tvdeyen/solidus-asset-override-demo works 👍 |
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.
Yea, this has tripped people up for an incredibly long time, good by me thanks.
This actually needs to be deployed somewhere in order to confirm its really working. My demo app is a proof though, but I want to be extra sure 😎
|
This came out of discussion with @tvdeyen
When we use
require_tree
or a relative path torequire
, it makes it so that attempting to override these files in your own app won't work.This PR replaces all
require_relative
calls with an individual hardcodedrequire
per-file. Requires for templates were split into their ownindex.js
fileFixes #1567