1
0
Fork 0
mirror of https://github.com/thoughtbot/factory_bot_rails.git synced 2022-11-09 11:49:18 -05:00
Commit graph

5 commits

Author SHA1 Message Date
Daniel Colson
bfee5d8dcc Set up reloading after_initialize
Alternate fix for #336. We went with #347 instead because the solution
in this commit didn't work with Rails 4.2. Since we are no longer
supporting Rails 4.2, I think this is a better approach.

The original problem 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

Triggering the first factory_bot reload before initializing I18n could
cause an error in some cases.

We avoided the problem in #347 by adding a conditional to skip reloading
factory_bot before the application has initialized.

This commit, on the other hand, moves factory_bot reloading from a
prepare callback into an `after_initialize` callback. The initialization
process is now simplified to:

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. factory_bot loads definitions for the first time. It then runs the
reloader to set up the prepare callback to reload factory_bot when the
application reloads (for example by calling `reload!` in the console),
and to register the reloader to trigger reloads when any factory_bot
definition files change.

[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
2020-06-15 11:15:46 -04:00
Daniel Colson
1e55d45bfb Ensure factory_bot only loads after initialization
Fixes #336
Alternate solution to #343

The initialization process looks 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 changes step 4 above to avoid reloading factory_bot before
the application has initialized.

[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

Co-authored-by: Danny Garcia <dannygarcia.me@gmail.com>
2019-10-02 18:37:42 -04:00
yuuji.yaginuma
9b83f482b5 Do not register a reloader when file paths not exist
If all the paths specified in `FactoryBot.definition_file_paths` do not
exist, an empty array is passed to the reloader.

If an empty array is specified, reloader passes it to `Listen.to` as it
is. This causes the application root to be watched and `node_modules` to
become a target of listening.

The behavior when an empty array is passed to reloader depends on the
Rails side. Fixes to not register a reloader when file paths do not exist
consistent behavior regardless of Rails side.

Fixes #328.
2019-04-09 10:39:51 -04:00
Jacob Frautschi
dcfc9f6662 Always execute a reload when Rails triggers one (instead of just on update) to prevent doubly-defined constants errors in factories when using Spring
- https://github.com/thoughtbot/factory_bot_rails/issues/323
2019-04-05 13:34:15 -04:00
Daniel Colson
02a0f58487 Allow reloading of factory definitions
Closes #236

This commit uses ActiveSupport's FileUpdateChecker to allow reloading
FactoryBot of definitions whenever a file in
`FactoryBot.definition_file_paths` gets updated.
This is similar to reloading for
[I18n](ced104d579/activesupport/lib/active_support/i18n_railtie.rb (L60-L70))
and [react rails](83b6175460/lib/react/rails/railtie.rb (L35-L41))

This allows us to get rid of any Spring-specific logic in the railtie,
since [Spring hooks into the application
reloader](0c711ff10b/lib/spring/application.rb (L161)).

This partly solves #211, since we no longer call `FactoryBot.reload` at
all in `after_initialize`. Instead, we will only call it when one of the
files in `definition_file_paths` gets updated. I say it partly
solves #211 because once a definition file gets updated, reloading
would still give warnings about redefining any constants in the
definition files. I wonder how common it is to define constants in the
same file as factory definitions. It's probably better to keep constants
in the autoload path, allowing Rails to handle unloading and reloading
them in development. I would want to see some specific examples before
worrying too much about it.

I would also like to offer a better way to configure the definition file
paths (see #165 and #166) and possibly an option to opt out of loading
definitions at all (could help with issues like #192).

A couple of quirks here:
* the to_prepare block could potentially get
[called multiple times](https://github.com/rails/rails/issues/28108).
It shouldn't matter, since `execute_if_updated` is a no-op if no
updates have been made.

* I noticed that the first time I call `reload!` in the console the
factory definitions get reloaded even when I made no updates to the
definition files. I think it is related to
[this](f7c5a8ce26/activesupport/lib/active_support/evented_file_update_checker.rb (L18)). After the first call
`reload!` works as expected, only reloading the factory definitions
if the definition files were updated.

* Rails uses execute rather than execute_if_updated for the
[route
reloader](https://github.com/rails/rails/blob/master/railties/lib/rails/application/finisher.rb#L133)
and for the [watchable file
reloader](https://github.com/rails/rails/blob/master/railties/lib/rails/application/finisher.rb#L173).
This means that changes to factory definitions will cause reloading of
the whole application. I think this is fine.
2018-09-08 02:21:47 +00:00