Add assets.resolve_assets_in_css_urls configuration option to allow disabling AssetUrlProcessor
#489
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I think this is a good option, but I don't see reverting what's now been released and out in the wild. If the 3.4.x series breaks things for you, you can set this option to avoid having the processor applied. But otherwise it's applied. |
We're introducing a new configuration option `assets.rewrite_css_urls` to allow users to opt out of the new `AssetUrlProcessor` (by configuring that option to `false`).
rmacklin
force-pushed
the
add-config-to-make-AssetUrlProcessor-opt-in
branch
from
f3c88fb to
a94fd77
Compare
|
Ok, I dropped that change, added tests, and took the PR out of draft state. |
|
This is looking good. We should document the option as well. Also, I think going with |
to use language that mirrors the new `resolve_assets_in_css_urls` configuration option
rmacklin
changed the title
assets.rewrite_css_urls configuration option to make AssetUrlProcessor opt-inassets.resolve_assets_in_css_urls configuration option to make AssetUrlProcessor opt-in
rmacklin
changed the title
assets.resolve_assets_in_css_urls configuration option to make AssetUrlProcessor opt-inassets.resolve_assets_in_css_urls configuration option to allow AssetUrlProcessor to be disabled
rmacklin
changed the title
assets.resolve_assets_in_css_urls configuration option to allow AssetUrlProcessor to be disabledassets.resolve_assets_in_css_urls configuration option to allow disabling AssetUrlProcessor
|
Ok, renamed the option to |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As reported in #478 / #486, the introduction of the
AssetUrlProcessorin #476 broke some folks' CSS assets when they upgraded beyond sprockets-rails v3.2.2, but it was not released in a new major version.To remedy this, let's makeAssetUrlProcessorbe opt-in for now with the plan to change it to opt-out in the next major version. In this PR, I'm proposing the addition of a newassets.rewrite_css_urlsconfiguration option to control whether or notAssetUrlProcessoris registered. We can default it tofalsein sprockets-rails v3.x and change the default totruein v4.0.Per #489 (comment), we'll roll forward with defaulting
AssetUrlProcessorto enabled, but it can be disabled by setting a newassets.resolve_assets_in_css_urlsconfiguration option tofalse.