try app_middleware instead of middleware #386
Conversation
|
Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @arthurnn (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
|
| @@ -32,7 +32,7 @@ class Railtie < ::Rails::Railtie | |||
|
|
|||
| # Remove the sass middleware if it gets inadvertently enabled by applications. | |||
| config.after_initialize do |app| | |||
There was a problem hiding this comment.
The proper fix is to change the middleware stack inside the initialization process. Can you change this block to a initializer?
This is a re-application of the change originally attempted in rails/sass-rails#386
Fixes #385, #136 (
cannot modify frozen Arrayissues).I don't know if this is the "proper" fix for these issues, but it's enough of a workaround for my particular use-case that I thought I'd share to get the ball rolling.
This change doesn't seem to fail any tests, but my guess is that this is due to a lack of test coverage on this particular use case rather than unchanged behavior, so perhaps some followup is still needed.