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

Move infraestructure code to lib #41

Closed
wants to merge 1 commit into from
Closed

Move infraestructure code to lib #41

wants to merge 1 commit into from

Conversation

fxn
Copy link
Contributor

@fxn fxn commented Dec 26, 2020

Fixes #15.

Introduction

This engine provides two kinds of code:

  1. Application-level code like Turbo::Native::NavigationController.
  2. Infraestructure-level code like Turbo::Broadcastable, used to decorate Rails classes in the initializers.

Application-level code is added to the autoload paths by Rails, and it is not only autoloadable, but also reloadable.

Infraestructure-level code should not be reloadable, because if you decorate ActiveRecord::Base including a module, editing the source code of that module won't change the decoration.

Infraestructure, as in railties or other gems, goes in lib.

Refactor 1

First refactor would be to move infra to lib and let file paths reflect constant paths as usual. That leaves a directory that looks like this:

lib/turbo
lib/turbo/broadcastable.rb
lib/turbo/frames_helper.rb
lib/turbo/native/navigation.rb
lib/turbo/test_assertions.rb
...

and we need to issue manual require calls.

We lose the neat organization that app provided, that is a step back for my taste, so let's revise that.

Refactor 2

We can bring that organization back introducing folders:

lib/turbo/controllers
lib/turbo/helpers
lib/turbo/models
lib/turbo/test

That is better in my view, but we still need manual require calls.

Refactor 3

That was precisely one of the things Zeitwerk wanted to address. We can define a loader that takes care of lib and forget about require.

Additionally, Zeitwerk supports organizational folders like the ones above via the collapse method. So we are all set.

The parent application loader still manages app, and they do not interfere with each other. Also, the parent application can run in classic mode and it works (but please do not do that in 6.1!).

@dhh
Copy link
Member

dhh commented Dec 28, 2020

Merged fix in form of #51

@dhh dhh closed this Dec 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

bin/rails zeitwerk:check displays deprecation warning on Rails 6.0.3.4
2 participants