mirror of
https://github.com/thoughtbot/factory_bot_rails.git
synced 2022-11-09 11:49:18 -05:00
Set up reloading after_initialize
Fixes #336 Before this commit, the initialization process looked like this: Rails runs all of the initializers 1. Run the ["factory_bot.set_factory_paths"][set_factory_paths] initializer 2. Run the ["factory_bot.register_reloader"][register_reloader] initializer, which sets up a [prepare callback][] 3. Run the [`:run_prepare_callbacks`][] initializer 4. This triggers the factory_bot [prepare callback][], which causes factory\_bot to [reload][] Rails runs `after_initialize` callbacks 1. [I18n initializes] 2. factory\_bot [reloads again][] as described in #334 The double reloading of factory_bot in this initialization is not ideal, but also shouldn't generally cause any problems on its own. The problems people are having in #336 come from the fact that I18n gets set up in an `after_initialize` callback, but factory_bot gets reloaded before the `after_initialize` callbacks are triggered. If the `FactoryBot.define` block references any code that uses I18n translations as it loads, that code will raise an error (references inside other factory_bot methods, or code that uses I18n translations inside of methods still works fine, since the whole Rails initialization process would be complete by the time any of that code runs). This commit moves factory_bot reloading from a prepare callback into an `after_initialize` callback. This allows us to avoid reloading factory_bot before I18n is loaded, and also gets rid of that pesky double reloading of factory_bot. The new initialization process looks like: Rails runs all of the initializers 1. Run the ["factory_bot.set_factory_paths"][set_factory_paths] 2. Run the [`:run_prepare_callbacks`][] initializer, which no longer involves factory_bot Rails runs `after_intialize` callbacks 1. [I18n initializes] 2. Run the factory_bot reloader, which sets up the prepare callback for any future prepares (for example calling `reload!` in the console), and executes the reloader to run the initial `FactoryBot.reload` [set_factory_paths]:3815aae2b9/lib/factory_bot_rails/railtie.rb (L17-L19)
[register_reloader]:3815aae2b9/lib/factory_bot_rails/railtie.rb (L21-L23)
[prepare callback]: https://github.com/thoughtbot/factory_bot_rails/blob/master/lib/factory_bot_rails/reloader.rb#L34-L36 [`:run_prepare_callbacks`]: https://github.com/rails/rails/blob/5-2-stable/railties/lib/rails/application/finisher.rb#L62-L64 [reload]: https://github.com/thoughtbot/factory_bot_rails/blob/master/lib/factory_bot_rails/reloader.rb#L24-L26 [I18n initializes]:13e2102517/activesupport/lib/active_support/i18n_railtie.rb (L16-L20)
[reloads again]: https://github.com/thoughtbot/factory_bot_rails/blob/master/lib/factory_bot_rails/railtie.rb#L25-L27
This commit is contained in:
parent
064838c0e0
commit
35bb9409aa
3 changed files with 25 additions and 22 deletions
|
@ -18,12 +18,8 @@ module FactoryBotRails
|
|||
FactoryBot.definition_file_paths = definition_file_paths
|
||||
end
|
||||
|
||||
initializer "factory_bot.register_reloader" do |app|
|
||||
Reloader.new(app, config).run
|
||||
end
|
||||
|
||||
config.after_initialize do
|
||||
FactoryBot.reload
|
||||
config.after_initialize do |app|
|
||||
Reloader.new(app).run
|
||||
end
|
||||
|
||||
private
|
||||
|
|
|
@ -4,21 +4,22 @@ require "factory_bot_rails/definition_file_paths"
|
|||
|
||||
module FactoryBotRails
|
||||
class Reloader
|
||||
def initialize(app, config)
|
||||
def initialize(app)
|
||||
@app = app
|
||||
@config = config
|
||||
@paths = DefinitionFilePaths.new(FactoryBot.definition_file_paths)
|
||||
end
|
||||
|
||||
def run
|
||||
return unless @paths.any?
|
||||
|
||||
register_reloader(build_reloader)
|
||||
reloader = build_reloader
|
||||
register_reloader(reloader)
|
||||
reloader.execute
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
attr_reader :app, :config
|
||||
attr_reader :app
|
||||
|
||||
def build_reloader
|
||||
reloader_class.new(@paths.files, @paths.directories) do
|
||||
|
@ -31,11 +32,11 @@ module FactoryBotRails
|
|||
end
|
||||
|
||||
def register_reloader(reloader)
|
||||
config.to_prepare do
|
||||
app.reloaders << reloader
|
||||
|
||||
app.reloader.to_prepare do
|
||||
reloader.execute
|
||||
end
|
||||
|
||||
app.reloaders << reloader
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -12,8 +12,6 @@ describe FactoryBotRails::Reloader do
|
|||
|
||||
context "when a definition file paths exist" do
|
||||
it "registers a reloader" do
|
||||
allow(reloader_class).to receive(:new)
|
||||
|
||||
run_reloader(["spec/fixtures/factories", "not_exist_directory"])
|
||||
|
||||
expect(reloader_class).to have_received(:new)
|
||||
|
@ -22,8 +20,6 @@ describe FactoryBotRails::Reloader do
|
|||
|
||||
context "when a file exists but not a directory" do
|
||||
it "registers a reloader" do
|
||||
allow(reloader_class).to receive(:new)
|
||||
|
||||
run_reloader(["spec/fake_app", "not_exist_directory"])
|
||||
|
||||
expect(reloader_class).to have_received(:new)
|
||||
|
@ -32,8 +28,6 @@ describe FactoryBotRails::Reloader do
|
|||
|
||||
context "when a definition file paths NOT exist" do
|
||||
it "does NOT register a reloader" do
|
||||
allow(reloader_class).to receive(:new)
|
||||
|
||||
run_reloader(["not_exist_directory"])
|
||||
|
||||
expect(reloader_class).not_to have_received(:new)
|
||||
|
@ -42,12 +36,24 @@ describe FactoryBotRails::Reloader do
|
|||
|
||||
def run_reloader(definition_file_paths)
|
||||
FactoryBot.definition_file_paths = definition_file_paths
|
||||
FactoryBotRails::Reloader.
|
||||
new(Rails.application, Rails.application.config).run
|
||||
FactoryBotRails::Reloader.new(app).run
|
||||
end
|
||||
|
||||
def app
|
||||
double(
|
||||
:app,
|
||||
config: double(:config, file_watcher: reloader_class),
|
||||
reloader: double(:reloader, to_prepare: nil),
|
||||
reloaders: [],
|
||||
)
|
||||
end
|
||||
|
||||
def reloader_class
|
||||
Rails.application.config.file_watcher
|
||||
@reloader_class ||=
|
||||
double(
|
||||
:reloader_class,
|
||||
new: double(:reloader, execute: nil),
|
||||
)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue