Translating filenames into class names on app eager loading fails for plural class names

jerryskye commented 5 years ago

Hello! Thanks for Jets, I absolutely love it ❤️ I noticed a problem while trying to run jets console in my application, here's a quick rundown of it 👇

Checklist

[x] Upgrade Jets: Are you using the latest version of Jets? This allows Jets to fix issues fast. There's a jets upgrade command that makes this a simple task. There's also an Upgrading Guide: http://rubyonjets.com/docs/upgrading/
[x] Reproducibility: Are you reporting a bug others will be able to reproduce and not asking a question. If you're unsure or want to ask a question, do so on https://community.rubyonjets.com
[x] Code sample: Have you put together a code sample to reproduce the issue and make it available? Code samples help speed up fixes dramatically. If it's an easily reproducible issue, then code samples are not needed. If you're unsure, please include a code sample.

Expected Behaviour

I can define a class with a plural name, like Quotes and have no problem with eager loading.

Current Behavior

The framework prevents me from having classes with plural names in my application if I want to eager load it.

Step-by-step reproduction instructions

echo 'class Quotes; end' > app/models/quotes.rb
jets console

Get an error from constantizing a singular class name, because the application defined it as plural:

Traceback (most recent call last):
    16: from /Users/hello/.rvm/gems/ruby-2.5.3/bin/jets:23:in `<main>'
    15: from /Users/hello/.rvm/gems/ruby-2.5.3/bin/jets:23:in `load'
    14: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/jets-1.8.9/exe/jets:14:in `<top (required)>'
    13: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/jets-1.8.9/lib/jets/cli.rb:5:in `start'
    12: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/jets-1.8.9/lib/jets/cli.rb:20:in `start'
    11: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/jets-1.8.9/lib/jets/cli.rb:48:in `boot_jets'
    10: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/jets-1.8.9/lib/jets/core.rb:18:in `boot'
     9: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/jets-1.8.9/lib/jets/booter.rb:24:in `boot!'
     8: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/jets-1.8.9/lib/jets/booter.rb:201:in `eager_load_app'
     7: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/jets-1.8.9/lib/jets/booter.rb:201:in `select'
     6: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/jets-1.8.9/lib/jets/booter.rb:218:in `block in eager_load_app'
     5: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/activesupport-5.2.2.1/lib/active_support/core_ext/string/inflections.rb:68:in `constantize'
     4: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/activesupport-5.2.2.1/lib/active_support/inflector/methods.rb:281:in `constantize'
     3: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/activesupport-5.2.2.1/lib/active_support/inflector/methods.rb:281:in `inject'
     2: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/activesupport-5.2.2.1/lib/active_support/inflector/methods.rb:281:in `each'
     1: from /Users/hello/.rvm/gems/ruby-2.5.3/gems/activesupport-5.2.2.1/lib/active_support/inflector/methods.rb:283:in `block in constantize'
/Users/hello/.rvm/gems/ruby-2.5.3/gems/activesupport-5.2.2.1/lib/active_support/inflector/methods.rb:283:in `const_get': uninitialized constant Quote (NameError)

Solution Suggestion

I believe the solution is to mimic the behavior from the eager_load_jets method and using camelize instead of classify in the eager_load_app method here.

tongueroo commented 5 years ago

RE: Thanks for Jets, I absolutely love it ❤️

Thanks for the kind words.

Thinking defining a custom inflection here may help you http://rubyonjets.com/docs/custom-inflections/

jerryskye commented 5 years ago

Hey, a custom inflection did indeed help me, thanks a lot 👍 I still think we should really switch to camelize here though :grin: I don't think classify should be used for translating filenames to class names, I guess ActiveSupport's documentation says it best, defining classify as a method for creating class names from database table names, following the convention of plural and underscored table names and camel cased singular classes. What do you think? BTW I grepped through the source code and found some more potential places where classify is used for this purpose. I was going to swap them all for camelize and run the test suite, but I encountered this error:

An error occurred while loading spec_helper.
Failure/Error: require "jets-gems"

LoadError:
  cannot load such file -- jets-gems
# /Users/hello/.rvm/gems/ruby-2.5.3@jets-test/gems/activesupport-5.2.3/lib/active_support/dependencies.rb:291:in `require'
# /Users/hello/.rvm/gems/ruby-2.5.3@jets-test/gems/activesupport-5.2.3/lib/active_support/dependencies.rb:291:in `block in require'
# /Users/hello/.rvm/gems/ruby-2.5.3@jets-test/gems/activesupport-5.2.3/lib/active_support/dependencies.rb:257:in `load_dependency'
# /Users/hello/.rvm/gems/ruby-2.5.3@jets-test/gems/activesupport-5.2.3/lib/active_support/dependencies.rb:291:in `require'
# ./lib/jets.rb:66:in `<top (required)>'
# ./spec/spec_helper.rb:16:in `require'
# ./spec/spec_helper.rb:16:in `<top (required)>'

There are some more errors, but I think they're caused by this LoadError above.

Thanks for your help and have a nice day! :relaxed: Stay awesome! 💎

tongueroo commented 5 years ago

@jerryskye Oh interesting! I thought that classes generally get converted to their class name equivalent with classify and didn't realize that classify was only supposed to be table names. 🤦🏻‍♂️

If you can fix it that'll be amazing 🎉 If not, will take a look when got time 👌

RE: cannot load such file -- jets-gems

The jets-gem is currently package as a submodule. Here's how you get it:

git submodule init
git submodule update

Check that the vendor/jets-gems folder is not empty. Then you should be good to run the test suite.

jerryskye commented 5 years ago

Yeah sure, I'll fix it :wink: Wow, didn't notice the .gitmodules there :sweat_smile: Thanks! :relaxed:

tongueroo commented 5 years ago

Done in #266 Released in v1.9.5

rubyonjets / jets