-
Notifications
You must be signed in to change notification settings - Fork 196
Propshaft support #130
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
Propshaft support #130
Conversation
Generated by 🚫 Danger |
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 we want something more organized where we have assets/stylesheets/grape_swagger_rails/default
and assets/stylesheets/grape_swagger_rails/propshaft
, separate tests that exercise both with and without propshaft, etc.
ae22e8c
to
2dc3529
Compare
2dc3529
to
f7df633
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 don't think I'm understanding how we are choosing sprockets vs. propshaft.
That said, everything sprockets-specific should end up in a subfolder called sprockets, and everything propshaft in a folder called propshaft. Everywhere anything is required we want to inject the "sprockets" or "propshaft" name and not do if
s.
Also README and CHANGELOG will need updates.
bc07313
to
a445d08
Compare
@dblock I've updated this PR to remove the Sprockets initialization stuff (moved to #132) and focus purely on adding support for Propshaft. Unlike Sprockets, Propshaft is not a bundler. All it does is fingerprint assets for cache busting ( Propshaft's README does a good job of explaining what it does exactly: https://github.com/rails/propshaft/blob/main/README.md#propshaft This means that with Propshaft, it's not possible to include a single Unfortunately I don't think there's a good way of deduplicating the list of assets when both Sprockets and Propshaft need to be supported :( Propshaft does not require any special initialization, as it will automatically include assets from 3rd party gems as long as they follow Rails conventions ( |
a445d08
to
1bcf081
Compare
1bcf081
to
3792e3e
Compare
3792e3e
to
402a261
Compare
Merged, thanks Olivier! Want to help co-maintain this gem? Maybe make the next release? Email me dblock at dblock dot org if you have interest. |
Thanks for merging! And sure, I wouldn't mind helping maintain the gem :) Can you add my non-corporate account @olivierbellone as a maintainer? |
This PR adds support for Propshaft, the newer Rails asset pipeline (and the default pipeline as of Rails 8).
Basically:
application.css
file and all JS into a singleapplication.js
fileapplication.css
andapplication.js
if Propshaft is not being usedI renamed
screen.css.erb
toscreen-sprockets.css.erb
and added a newscreen-propshaft.css
file, since Propshaft does not support ERB templates for assets, but does support referencing images in CSS (cf. docs).This is the same approach used by other Rails engines such as blazer, e.g.:
I manually tested this with both Sprockets and Propshaft, and AFAICT everything works as expected.
Closes #129.