Make sure annotate_rendered_view_with_filenames is configured #130
Conversation
Action View is setup to copy the configuration from the application after initialization is finished, using `ActiveSupport.on_load(:action_view)` to make sure it only happens when `ActionView::Base` is loaded. We need to add our on_load hook after that, so we're also putting it in an after_initialize. Typically apps load all Rails railties before requiring gems with Bundler, so our after_initialize should always run after the one from Action View.
etiennebarrie
deleted the
fix-annotate_rendered_view_with_filenames-config
branch
|
|
||
| config.after_initialize do |
There was a problem hiding this comment.
Reverting this change I still get the test passing
There was a problem hiding this comment.
The test was working fine. When running test_helper, the first time Rails wasn't defined and lib/better_html.rb wouldn't require the railtie:
better-html/lib/better_html.rb
Line 27 in d2e71f5
But then when the specific test file would be loaded, we would make sure the railtie is loaded on its own. Since the actual config-copying code only runs when ActionView::Base is loaded, and that only happens later in the test with this hack, the initializer code copying the config executed and the test was checking that the config was indeed copied
Still your commit makes it better because the railtie will always be loaded before the dummy application is initialized, the way it's done in a real Rails app. In other words the tests don't rely on the fact that it all happens in an on_load(:action_view). 👍
|
Since nobody was able to explain to me why the old code didn't work, just that it didn't work, the reason is because |
|
The PR description and the commit message in ea19da1 tried to explain that. It's more complicated than that, the config from application to Active Record does:
While Action View does:
So for this gem here, not only do we have to copy the config after initialize, we have to copy the config in an on_load hook in the after initialize so that it runs after the hook that copies the configs. That's why the old code didn't work. |
|
@etiennebarrie Thanks for fixing this and sorry for breaking rails 6 in the first place |
This fixes two problems with #122.
First the configuration is properly copied (we needed to add our
on_loadhook in anafter_initializeso that it's added after the configuration is copied from the Rails application toActionView::Base). Replaces #128.Second, we only do this if the configuration exists, keeping compatibility with 6.0. Fixes #129.