Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change loader behaviour to support pre-Rails 6 apps #251

Merged
merged 1 commit into from
Mar 9, 2021

Conversation

paracycle
Copy link
Member

Motivation

Fixes: #239

Rails.autoloaders did not exist in Rails pre-6.0, so we should not assume that it will be there. We were faking this by defining an autoloaders method on the fake Rails module defined in our test repo, but we really should not be doing that.

Moreover, we were not actually trying to eager load a Rails app in a way that is compatible with pre-6.0 versions, which this commit aims to add as well.

Implementation

Mostly extra checks to see if methods are defined as we expect and a simplification of the test repo.

I've also added an eager loading method via config.eager_load_namespaces that should work in pre-6.0 apps as well. However, I am assuming that Rails.application.config will always exist in a sensible Rails app.

Tests

No extra tests.

`Rails.autoloaders` did not exist in Rails pre-6.0, so we should not
assume that it will be there. We were faking this by defining an
`autoloaders` method on the fake `Rails` module defined in our test
repo, but we really should not be doing that.

Moreover, we were not actually trying to eager load a Rails app in a way
that is compatible with pre-6.0 versions, which this commit aims to add
as well.

Fixes: #239
@paracycle paracycle requested a review from a team March 9, 2021 20:28
@paracycle paracycle merged commit eb81862 into master Mar 9, 2021
@paracycle paracycle deleted the uk-support-pre-rails-6 branch March 9, 2021 22:11
@paracycle paracycle temporarily deployed to production March 9, 2021 23:07 Inactive
This was referenced Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tapioca::Loader relies on Rails 6+
4 participants