fxn / zeitwerk

Efficient and thread-safe code loader for Ruby
MIT License
2k stars 118 forks source link

autoloader.inflector.inflect overrides not honoured by Rails #106

Closed EmmaB closed 4 years ago

EmmaB commented 4 years ago

Hello, and thanks for Zeitwerk and all the great docs!

We have an issue where Rails internals are not honouring inflector overrides. Here's a test-app.zip illustrating the issue, but here are the relevant bits:

# config/routes.rb
Rails.application.routes.draw do
  resources :ai_template
end
# app/controllers/ai_template_controller.rb
class AITemplateController < ApplicationController
  def index
  end
end
# Gemfile
ruby "2.6.5"
gem 'rails', "6.0.2.1"
gem 'puma', '~> 3.7'
# (no Spring or Turbolinks)

And here's how the inflections initializer looks, as per the docs:

# config/initalizers/inflections.rb
Rails.autoloaders.each do |autoloader|
  autoloader.inflector.inflect(
    "ai_template_controller" => "AITemplateController",
    "ai_template_helper"     => "AITemplateHelper"
  )
end

Here's the error on starting a server:

=> Booting Puma
=> Rails 6.0.2.1 application starting in development 
=> Run `rails server --help` for more startup options
Puma starting in single mode...
* Version 3.12.2 (ruby 2.6.5-p114), codename: Llamas in Pajamas
* Min threads: 5, max threads: 5
* Environment: development
* Listening on tcp://localhost:3000
Use Ctrl-C to stop
2020-02-04 11:10:20 +0000: Rack app error handling request { GET /ai_template }
#<NameError: uninitialized constant AiTemplateHelper
Did you mean?  AITemplateHelper>
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/inflector/methods.rb:282:in `const_get'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/inflector/methods.rb:282:in `block in constantize'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/inflector/methods.rb:280:in `each'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/inflector/methods.rb:280:in `inject'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/inflector/methods.rb:280:in `constantize'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/core_ext/string/inflections.rb:68:in `constantize'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/actionpack-6.0.2.1/lib/abstract_controller/helpers.rb:156:in `block in modules_for_helpers'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/actionpack-6.0.2.1/lib/abstract_controller/helpers.rb:144:in `map!'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/actionpack-6.0.2.1/lib/abstract_controller/helpers.rb:144:in `modules_for_helpers'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/actionpack-6.0.2.1/lib/action_controller/metal/helpers.rb:94:in `modules_for_helpers'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/actionpack-6.0.2.1/lib/abstract_controller/helpers.rb:108:in `helper'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/actionpack-6.0.2.1/lib/action_controller/railties/helpers.rb:19:in `inherited'
/Users/Emma/rails/test-app/app/controllers/application_controller.rb:1:in `<top (required)>'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/zeitwerk-2.2.2/lib/zeitwerk/kernel.rb:16:in `require'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/zeitwerk-2.2.2/lib/zeitwerk/kernel.rb:16:in `require'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/actiontext-6.0.2.1/lib/action_text/engine.rb:46:in `block (2 levels) in <class:Engine>'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/lazy_load_hooks.rb:72:in `class_eval'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/lazy_load_hooks.rb:72:in `block in execute_hook'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/lazy_load_hooks.rb:62:in `with_execution_control'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/lazy_load_hooks.rb:67:in `execute_hook'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/lazy_load_hooks.rb:52:in `block in run_load_hooks'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/lazy_load_hooks.rb:51:in `each'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/lazy_load_hooks.rb:51:in `run_load_hooks'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/actiontext-6.0.2.1/lib/action_text/content.rb:132:in `<top (required)>'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/zeitwerk-2.2.2/lib/zeitwerk/kernel.rb:23:in `require'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/zeitwerk-2.2.2/lib/zeitwerk/kernel.rb:23:in `require'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/actiontext-6.0.2.1/lib/action_text/engine.rb:42:in `block (2 levels) in <class:Engine>'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/callbacks.rb:429:in `instance_exec'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/callbacks.rb:429:in `block in make_lambda'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/callbacks.rb:201:in `block (2 levels) in halting'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/callbacks.rb:607:in `block (2 levels) in default_terminator'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/callbacks.rb:606:in `catch'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/callbacks.rb:606:in `block in default_terminator'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/callbacks.rb:202:in `block in halting'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/callbacks.rb:514:in `block in invoke_before'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/callbacks.rb:514:in `each'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/callbacks.rb:514:in `invoke_before'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/callbacks.rb:134:in `run_callbacks'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/execution_wrapper.rb:111:in `run!'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/execution_wrapper.rb:73:in `block in run!'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/execution_wrapper.rb:70:in `tap'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/activesupport-6.0.2.1/lib/active_support/execution_wrapper.rb:70:in `run!'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/actionpack-6.0.2.1/lib/action_dispatch/middleware/executor.rb:12:in `call'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/actionpack-6.0.2.1/lib/action_dispatch/middleware/static.rb:126:in `call'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/rack-2.1.2/lib/rack/sendfile.rb:113:in `call'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/actionpack-6.0.2.1/lib/action_dispatch/middleware/host_authorization.rb:83:in `call'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/railties-6.0.2.1/lib/rails/engine.rb:526:in `call'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/puma-3.12.2/lib/puma/configuration.rb:227:in `call'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/puma-3.12.2/lib/puma/server.rb:674:in `handle_request'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/puma-3.12.2/lib/puma/server.rb:476:in `process_client'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/puma-3.12.2/lib/puma/server.rb:334:in `block in run'
/Users/Emma/.rvm/gems/ruby-2.6.5/gems/puma-3.12.2/lib/puma/thread_pool.rb:135:in `block in spawn_thread'
^C- Gracefully stopping, waiting for requests to finish
=== puma shutdown: 2020-02-04 11:11:42 +0000 ===
- Goodbye!

The server starts fine if you add this to the initializer:

ActiveSupport::Inflector.inflections do |inflect|
  inflect.acronym "AI"
end

but this causes issues when, for instance, we need to titleize a string ("Taiwan", for instance).

We think it's a reasonable expectation that Rails should honour the explicit inflectors overrides, without needing to have a global ActiveSupport inflector as well. Do you think a bug, or is it how it's meant to work?


Just before I posted this issue, I noticed this comment in the rails new generated initializer.rb file, just above the ActiveSupport::Inflector bit: # need these for when active support needs to inflect, such as on helper class names.

So maybe this is a known issue, or not considered an issue? Or could it be more of a bug in the Rails titleize method? Any thoughts on how you'd work around this clash would be fantastic.

casperisfine commented 4 years ago

Seem related to https://github.com/rails/rails/pull/37632

fxn commented 4 years ago

Yep, I suspect we need to revise camelize usage in the framework, or document potential mismatches, or restrict the inflector interface....

@EmmaB thanks for reporting this. I have moved it to https://github.com/rails/rails/issues/38399 since Zeitwerk itself is fine, it is really an issue in the Rails integration.

kmlrj commented 1 year ago

@fxn I still have this issue.

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "activesupport", "6.0.3"
  gem "zeitwerk"
end

require "active_support"
require "active_support/core_ext/object/blank"
require "minitest/autorun"
require "zeitwerk"

class BugTest < Minitest::Test
  def test_acronym_override_1
    # In rails projects, I use `Rails.autoloaders.main.inflector.inflect`
    Zeitwerk::Loader.new.inflector.inflect(
      "ai_template_controller" => "AITemplateController"
    )

    assert_equal("AITemplateController", ActiveSupport::Inflector.camelize("ai_template_controller"))
    assert_equal("Taiwan", ActiveSupport::Inflector.titleize("taiwan"))
  end

  # def test_acronym_override_2
  #   ActiveSupport::Inflector.inflections do |inflect|
  #     inflect.acronym("AI")
  #   end

  #   Zeitwerk::Loader.new.inflector.inflect(
  #     "ai_template_controller" => "AiTemplateController"
  #   )

  #   assert_equal("AiTemplateController", ActiveSupport::Inflector.camelize("ai_template_controller"))
  #   assert_equal("Taiwan", ActiveSupport::Inflector.titleize("taiwan"))
  # end
end
fxn commented 1 year ago

@kmlrj hmmm, what do you mean? That test does not show a bug, it is working as expected.

A new Zeitwerk loader uses its own inflector. And that inflector does not use Active Support. Zeitwerk does not have Active Support as a gem dependency.

In Rails applications, Rails itself changes the default inflector of the autolaoders with one that uses AS. If you want things to match, you need to configure Active Support as explained in the guide.