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

Add simple Zeitwerk integration #1100

Merged
merged 12 commits into from
Apr 30, 2021

Conversation

timriley
Copy link
Member

@timriley timriley commented Apr 14, 2021

Set up autoloading by default for Hanami 2 applications.

This uses Zeitwerk and dry-system's autoloading support.

To support this, we do the following:

  • Set up a Zeitwerk loader by default at Hanami.application.configuration.autoloader
  • When the application and slice containers are prepared:
    • Add their directories to the Zeitwerk loader
    • Enable the dry-system Loader::Autoloading and disable the containers' component dirs from being added to the load path
  • Then later in the Hanami app init process
    • Configure the autoloader with the app's inflector (via an adapter, since the expected API is slightly different)
    • Then call #setup on the loader, effectively finalizing its config

We also support opting out of autoloading entirely by setting Hanami.application.configuration.autoloader to nil or false.

@timriley timriley force-pushed the enhancement/unstable/configure-zeitwerk-first-pass branch from dc846b1 to f24a5d0 Compare April 14, 2021 13:52
@timriley
Copy link
Member Author

@jodosha jodosha added this to the v2.0.0 milestone Apr 14, 2021
@fxn
Copy link
Contributor

fxn commented Apr 14, 2021

That is all? Man, I do not know the details but this patch tells me the work behind this integration is really good 💯.

@timriley
Copy link
Member Author

@fxn Thanks for taking a look! ❤️

As my latest commit shows (c35c7ae), there's still a little to come, but I hope the integration picture overall won't need to become much more complex than it is now.

If there's any necessary part of integration that you think we might be overlooking, would definitely appreciate any pointers!

One thing I'm keeping in mind is enabling reloading when the application is in development mode, but I think I might consider that in another PR, after all the basic integration is done and shown to be working smoothly.

@timriley timriley marked this pull request as ready for review April 29, 2021 14:10
Copy link
Member

@jodosha jodosha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timriley Great to see this to happen! 👏

Copy link
Member

@solnic solnic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things are getting real 💪🏻

Base automatically changed from enhancement/unstable/configure-dry-system-component-dirs to unstable April 30, 2021 10:58
@timriley timriley force-pushed the enhancement/unstable/configure-zeitwerk-first-pass branch from 02d1563 to 0fb38bf Compare April 30, 2021 11:04
@timriley timriley merged commit d044674 into unstable Apr 30, 2021
@timriley timriley deleted the enhancement/unstable/configure-zeitwerk-first-pass branch April 30, 2021 11:06
@fxn
Copy link
Contributor

fxn commented Apr 30, 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.

5 participants