Commit 68537b2c authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Clean up unneeded code and docs

Since we switched to the Zeitwerk autoloader, these are no longer
necessary
parent 40a2b67e
...@@ -268,22 +268,6 @@ static-analysis as-if-foss: ...@@ -268,22 +268,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