Automatically register assets with Sprockets #132
Conversation
There was a problem hiding this comment.
CI won't pass until #131 is merged and this branch is rebased.
| ### Enabling in a Rails-API Project | ||
|
|
||
| The grape-swagger-rails gem uses the Rails asset pipeline for its Javascript and CSS. Enable the asset pipeline with [rails-api](https://github.com/rails-api/rails-api). |
There was a problem hiding this comment.
Rails now natively supports API-only mode and the rails-api repo has long gone stale, so this entire section is no longer relevant.
| ### Enabling in Rails 6 (Sprokets 5) | ||
|
|
||
| Rails 6 top-level targets are determined via `./app/assets/config/manifest.js`. Specify `grape-swagger-rails` asset files as follows. | ||
|
|
||
| ```javascript | ||
| //= link grape_swagger_rails/application.css | ||
| //= link grape_swagger_rails/application.js | ||
| ``` | ||
|
|
||
| See [Upgrading Sprokets](https://github.com/rails/sprockets/blob/master/UPGRADING.md#manifestjs) for more information. | ||
|
|
There was a problem hiding this comment.
This is no longer necessary, as the Railtie will now automatically register assets with Sprockets.
| @@ -1,3 +1,2 @@ | |||
| //= link_directory ../stylesheets .css | |||
| //= link_directory ../javascripts .js | |||
| //= link grape_swagger_rails_manifest.js | |||
There was a problem hiding this comment.
I verified that the test suite doesn't pass without the change in this PR when this line is removed, and does pass with the change.
| app.config.assets.precompile << proc { |path| path == 'grape_swagger_rails/application.js' } | ||
| app.config.assets.precompile << proc { |path| path == 'grape_swagger_rails/application.css' } | ||
| app.config.assets.precompile << proc { |path| path == 'grape_swagger_rails/favicon.ico' } | ||
| end |
There was a problem hiding this comment.
Nitpicking, maybe this is better? Don't feel strongly about it at all.
PATHS = ['grape_swagger_rails/application.js', ...]
app.config.assets.precompile.concat Gem::Version.new(Sprockets::VERSION) >= Gem::Version.new("4.0.0") ? paths : proc { |path| PATHS.include?(path) }There was a problem hiding this comment.
I've tweaked the code to avoid duplicating the list of asset paths, lmk what you think!
olivier-thatch
force-pushed
the
sprockets-initializer
branch
2 times, most recently
from
84839be to
ffaadd4
Compare
I'm not sure it's worth adding a new dimension to the matrix just for this. Nevertheless, I've added support for specifying the Sprockets version via the If you'd like to proceed with adding this to the CI matrix, let's merge #133 first as it will make it a lot easier (one line vs. a bunch of new entries). |
|
I merged #133, want to rebase? |
olivier-thatch
force-pushed
the
sprockets-initializer
branch
2 times, most recently
from
f3f771d to
b22e05c
Compare
There was a problem hiding this comment.
Increment version in https://github.com/ruby-grape/grape-swagger-rails/blob/master/lib/grape-swagger-rails/version.rb as well.
olivier-thatch
force-pushed
the
sprockets-initializer
branch
from
b22e05c to
834107f
Compare
|
@olivier-thatch Do you know what the issue in the HEAD tests is? |
|
@dblock Not sure, but I think these are transient failures. I'm pretty sure the tests were passing before I updated |
olivier-thatch
force-pushed
the
sprockets-initializer
branch
from
834107f to
a180cdb
Compare
Retrying those fails them again, but I don't understand why the error happens either. Let's look at it later. |
Automatically register assets with Sprockets.
This removes the need for users of the gem to add
//= link grape_swagger_rails_manifest.{css,js}to their ownapp/assets/config/manifest.js.