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

Zeitwerk breaks override pattern #36100

Closed
jasl opened this issue Apr 26, 2019 · 7 comments
Closed

Zeitwerk breaks override pattern #36100

jasl opened this issue Apr 26, 2019 · 7 comments

Comments

@jasl
Copy link
Contributor

@jasl jasl commented Apr 26, 2019

Steps to reproduce

Follow https://edgeguides.rubyonrails.org/engines.html#overriding-models-and-controllers create an override (monkey patch) for a class

This works before 6.0.0.rc1 (including 6.0.0.beta2, 6.0.0.beta3)

Expected behavior

Class should be patched

Actual behavior

/Users/jasl/.rvm/gems/ruby-2.6.3/gems/zeitwerk-2.1.5/lib/zeitwerk/loader/callbacks.rb:15:in `on_file_autoloaded': expected file /Users/jasl/Workspaces/Ruby/cybros/app/overrides/doorkeeper/application_override.rb to define constant Doorkeeper::ApplicationOverride, but didn't (NameError)

System configuration

Rails version: 6.0.0.rc1

Ruby version: 2.6.3

@jasl

This comment has been minimized.

Copy link
Contributor Author

@jasl jasl commented Apr 26, 2019

@fxn could you please to give some advice?

@fxn

This comment has been minimized.

Copy link
Member

@fxn fxn commented Apr 26, 2019

Could you please try adding this?

# config/application.rb
load_defaults "6.0"
Rails.autoloaders.main.ignore("#{Rails.root}/app/overrides")
@jasl

This comment has been minimized.

Copy link
Contributor Author

@jasl jasl commented Apr 26, 2019

@fxn
Thank you, It works!
I'll send a PR to update the doc

@jasl jasl closed this Apr 26, 2019
@fxn

This comment has been minimized.

Copy link
Member

@fxn fxn commented Apr 26, 2019

Awesome!

Eric-Guo added a commit to thape-cn/cybros_portal that referenced this issue Apr 26, 2019
tavy88 added a commit to DFE-Digital/get-help-to-retrain that referenced this issue Sep 2, 2019
Zeitwerk breaks the override pattern e.g expected file
/get-help-to-retrain/app/admin/categories.rb to define constant
Categories, but didn't (NameError), we need to ignore the
exceptions.

This approach clashes with ActiveAdmin in our case, as all related
files live under /admin but do not have the expected structure.

The fix is to ignore the files that break this expected behaviour,
by doing something like:

Rails.autoloaders.main.ignore("#{Rails.root}/app/admin")

Resource:
rails/rails#36100
tavy88 added a commit to DFE-Digital/get-help-to-retrain that referenced this issue Sep 2, 2019
Zeitwerk breaks the override pattern e.g expected file
/get-help-to-retrain/app/admin/categories.rb to define constant
Categories, but didn't (NameError), we need to ignore the
exceptions.

This approach clashes with ActiveAdmin in our case, as all related
files live under /admin but do not have the expected structure.

The fix is to ignore the files that break this expected behaviour,
by doing something like:

Rails.autoloaders.main.ignore("#{Rails.root}/app/admin")

Resource:
rails/rails#36100
@aleksandrs-ledovskis

This comment has been minimized.

Copy link

@aleksandrs-ledovskis aleksandrs-ledovskis commented Oct 9, 2019

I'll send a PR to update the doc

Didn't seem to happen. @fxn's suggested fix needs broader exposure as Rails.autoloaders.main.ignore isn't adequately documented anywhere else.

@jasl

This comment has been minimized.

Copy link
Contributor Author

@jasl jasl commented Oct 10, 2019

I'll send a PR to update the doc

Didn't seem to happen. @fxn's suggested fix needs broader exposure as Rails.autoloaders.main.ignore isn't adequately documented anywhere else.

It happened long ago, but didn't merge yet, see #36102

@aleksandrs-ledovskis

This comment has been minimized.

Copy link

@aleksandrs-ledovskis aleksandrs-ledovskis commented Oct 10, 2019

@jasl Thanks for pointing out the link I've missed

fabianrbz pushed a commit to Nexmo/nexmo-developer that referenced this issue Oct 22, 2019
It was causing it to fail in production because of the folder structure
and what was expected to be defined. See rails/rails#36100 (comment)
for more info.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.