Commit fc39029d authored by Lin Jen-Shin's avatar Lin Jen-Shin

Merge branch 'zeitwerk-cleanup' into 'master'

Clean up unneeded code and docs

See merge request gitlab-org/gitlab!68401
parents 7931ee16 68537b2c
...@@ -272,22 +272,6 @@ static-analysis as-if-foss: ...@@ -272,22 +272,6 @@ static-analysis as-if-foss:
- .static-analysis:rules:as-if-foss - .static-analysis:rules:as-if-foss
- .as-if-foss - .as-if-foss
zeitwerk-check:
extends:
- .rails-cache
- .default-before_script
- .rails:rules:ee-and-foss-unit
variables:
BUNDLE_WITHOUT: ""
SETUP_DB: "false"
needs: []
stage: test
script:
- sed -i -e "s/config\.autoloader = :classic/config\.autoloader = :zeitwerk/" config/application.rb
- RAILS_ENV=test bundle exec rake zeitwerk:check
- RAILS_ENV=development bundle exec rake zeitwerk:check
- RAILS_ENV=production bundle exec rake zeitwerk:check
rspec migration pg12: rspec migration pg12:
extends: extends:
- .rspec-base-pg12 - .rspec-base-pg12
......
...@@ -58,39 +58,7 @@ module InjectEnterpriseEditionModule ...@@ -58,39 +58,7 @@ module InjectEnterpriseEditionModule
end end
def const_get_maybe_false(mod, name) def const_get_maybe_false(mod, name)
# We're still heavily relying on Rails autoloading instead of zeitwerk, mod && mod.const_defined?(name, false) && mod.const_get(name, false)
# therefore this check: `mod.const_defined?(name, false)`
# Is not reliable, which may return false while it's defined.
# After we moved everything over to zeitwerk we can avoid rescuing
# NameError and just check if const_defined?
# mod && mod.const_defined?(name, false) && mod.const_get(name, false)
result = mod && mod.const_get(name, false)
if result.name == "#{mod}::#{name}"
result
else
# This may hit into a Rails issue that when we try to load
# `EE::API::Appearance`, Rails might load `::Appearance` the first time
# when `mod.const_get(name, false)` is called if `::Appearance` is not
# loaded yet. This can be demonstrated as the following:
#
# EE.const_get('API::Appearance', false) # => Appearance
# EE.const_get('API::Appearance', false) # => raise NameError
#
# Getting a `NameError` is what we're expecting here, because
# `EE::API::Appearance` doesn't exist.
#
# This is because Rails will attempt to load constants from all the
# parent namespaces, and if it finds one it'll load it and return it.
# However, the second time when it's called, since the top-level class
# is already loaded, then Rails will skip this process. This weird
# behaviour can be worked around by calling this the second time.
# The particular line is at:
# https://github.com/rails/rails/blob/v6.1.3.2/activesupport/lib/active_support/dependencies.rb#L569-L570
mod.const_get(name, false)
end
rescue NameError
false
end end
end end
......
...@@ -203,76 +203,6 @@ in an initializer. ...@@ -203,76 +203,6 @@ in an initializer.
- Stack Overflow: [Why you should not write inline JavaScript](https://softwareengineering.stackexchange.com/questions/86589/why-should-i-avoid-inline-scripting) - Stack Overflow: [Why you should not write inline JavaScript](https://softwareengineering.stackexchange.com/questions/86589/why-should-i-avoid-inline-scripting)
## Auto loading
Rails auto-loading on `development` differs from the load policy in the `production` environment.
In development mode, `config.eager_load` is set to `false`, which means classes
are loaded as needed. With the classic Rails autoloader, it is known that this can lead to
[Rails resolving the wrong class](https://guides.rubyonrails.org/v5.2/autoloading_and_reloading_constants.html#when-constants-aren-t-missed-relative-references)
if the class name is ambiguous. This can be fixed by specifying the complete namespace to the class.
### Error prone example
```ruby
# app/controllers/application_controller.rb
class ApplicationController < ActionController::Base
...
end
# app/controllers/projects/application_controller.rb
class Projects::ApplicationController < ApplicationController
...
private
def project
...
end
end
# app/controllers/projects/submodule/some_controller.rb
module Projects
module Submodule
class SomeController < ApplicationController
def index
@some_id = project.id
end
end
end
end
```
In this case, if for any reason the top level `ApplicationController`
is loaded but `Projects::ApplicationController` is not, `ApplicationController`
would be resolved to `::ApplicationController` and then the `project` method is
undefined, causing an error.
#### Solution
```ruby
# app/controllers/projects/submodule/some_controller.rb
module Projects
module Submodule
class SomeController < Projects::ApplicationController
def index
@some_id = project.id
end
end
end
end
```
By specifying `Projects::`, we tell Rails exactly what class we are referring
to and we would avoid the issue.
NOTE:
This problem disappears as soon as we upgrade to Rails 6 and use the Zeitwerk autoloader.
### Further reading
- Rails Guides: [Autoloading and Reloading Constants (Classic Mode)](https://guides.rubyonrails.org/autoloading_and_reloading_constants_classic_mode.html)
- Ruby Constant lookup: [Everything you ever wanted to know about constant lookup in Ruby](https://cirw.in/blog/constant-lookup)
- Rails 6 and Zeitwerk autoloader: [Understanding Zeitwerk in Rails 6](https://medium.com/cedarcode/understanding-zeitwerk-in-rails-6-f168a9f09a1f)
## Storing assets that do not require pre-compiling ## Storing assets that do not require pre-compiling
Assets that need to be served to the user are stored under the `app/assets` directory, which is later pre-compiled and placed in the `public/` directory. Assets that need to be served to the user are stored under the `app/assets` directory, which is later pre-compiled and placed in the `public/` directory.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment