Fix ActionMailer preview loading #3901
Conversation
|
Is this seeking to resolve #3898? |
|
I believe it would as it would push load_previews' descendants call into runtime and out of intializer time which results in autoloading of the Preview classes. |
|
@jarednorman @PeteTheSadPanda yep, this solves #3898. It needs some more love — mostly, I want to implement some kind of caching/memoization and give that anonymous module its own place — but I've tested it in a real Solidus store and both Solidus' and the store's previews are loaded correctly. |
|
Given how frequently this gets called, I'm not sure caching/memorization is really needed. This call primarily gets called when rendering /rails/previews which itself does some level of caching by short circuiting is descendants are populated, so really you might only be requiring Solidus previews. I'm not advocating against your proposal which is fine, I'm just stating that if it proves too much a pain it wouldn't be a blocker (to me). I believe (but am not certain) that any subsequent calls to the same file via require_dependency would be cached assuming the constant is still in memory and hasn't been unloaded due to it being changed. |
Loading constants during an app's initialization is a deprecated technique and will be an error in future versions of Rails. By moving our custom logic for loading Solidus's previews to an `after_initialize` block, we ensure the preview classes are only loaded after initialization has completed.
|
This should be ready for a review! |
kennyadsl
added
Needs Backport
changelog:solidus_core
|
Failing specs already addressed, merging anyway! |
Description
This fixes #3898, by moving the loading of the app's previews (and our own) to an
after_initializeblock, which gets rid of the deprecation warning about constant loading during app initialization.I also managed to write a test that verifies that both the engine's and the app's previews are properly loaded. I had to enable preview loading in
Rails.env.test?as well in order for the test to pass, but I think that won't be a problem.Btw, it seems like we'll be able to get rid of this hack if/when rails/rails#31595 gets merged.
Checklist: