Skip to content

Commit

Permalink
Set up reloading after_initialize
Browse files Browse the repository at this point in the history
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]: https://github.com/thoughtbot/factory_bot_rails/blob/3815aae2b9e4a5c5c3027a2ee8851f44b7c6a4da/lib/factory_bot_rails/railtie.rb#L17-L19
[register_reloader]: https://github.com/thoughtbot/factory_bot_rails/blob/3815aae2b9e4a5c5c3027a2ee8851f44b7c6a4da/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]: https://github.com/rails/rails/blob/13e2102517fafc8f8736fce5d57de901067202d0/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
  • Loading branch information
composerinteralia committed Sep 20, 2019
1 parent 064838c commit 35bb940
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 22 deletions.
8 changes: 2 additions & 6 deletions lib/factory_bot_rails/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,8 @@ class Railtie < Rails::Railtie
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
Expand Down
15 changes: 8 additions & 7 deletions lib/factory_bot_rails/reloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,22 @@

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
Expand All @@ -31,11 +32,11 @@ def reloader_class
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
24 changes: 15 additions & 9 deletions spec/factory_bot_rails/reloader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@

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)
Expand All @@ -22,8 +20,6 @@

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)
Expand All @@ -32,8 +28,6 @@

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)
Expand All @@ -42,12 +36,24 @@

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

0 comments on commit 35bb940

Please sign in to comment.