ForestAdmin / forest-rails

💎 Ruby on Rails agent for Forest Admin to integrate directly to your existing Ruby on Rails backend application.
https://www.forestadmin.com
GNU General Public License v3.0
371 stars 76 forks source link

warnings in development and production #271

Closed ghost closed 5 years ago

ghost commented 6 years ago

I'm using ruby 2.5.1 and it seems I get new warnings I didn't have before. Any idea how to get ride of those?


/Users/daniel/.rvm/gems/ruby-2.5.1/gems/forest_liana-2.8.2/app/serializers/forest_liana/serializer_factory.rb:11: warning: already initialized constant ForestLiana::UserSpace::CustomerSerializer
/Users/daniel/.rvm/gems/ruby-2.5.1/gems/forest_liana-2.8.2/app/serializers/forest_liana/serializer_factory.rb:11: warning: previous definition of CustomerSerializer was here
/Users/daniel/.rvm/gems/ruby-2.5.1/gems/forest_liana-2.8.2/app/serializers/forest_liana/serializer_factory.rb:11: warning: already initialized constant ForestLiana::UserSpace::MapSerializer
/Users/daniel/.rvm/gems/ruby-2.5.1/gems/forest_liana-2.8.2/app/serializers/forest_liana/serializer_factory.rb:11: warning: previous definition of MapSerializer was here
/Users/daniel/.rvm/gems/ruby-2.5.1/gems/forest_liana-2.8.2/app/serializers/forest_liana/serializer_factory.rb:11: warning: already initialized constant ForestLiana::UserSpace::ModelSerializer
/Users/daniel/.rvm/gems/ruby-2.5.1/gems/forest_liana-2.8.2/app/serializers/forest_liana/serializer_factory.rb:11: warning: previous definition of ModelSerializer was here
/Users/daniel/.rvm/gems/ruby-2.5.1/gems/forest_liana-2.8.2/app/serializers/forest_liana/serializer_factory.rb:11: warning: already initialized constant ForestLiana::UserSpace::ReviewChannelSerializer
/Users/daniel/.rvm/gems/ruby-2.5.1/gems/forest_liana-2.8.2/app/serializers/forest_liana/serializer_factory.rb:11: warning: previous definition of ReviewChannelSerializer was here
/Users/daniel/.rvm/gems/ruby-2.5.1/gems/forest_liana-2.8.2/app/serializers/forest_liana/serializer_factory.rb:11: warning: already initialized constant ForestLiana::UserSpace::SharedAssetSerializer
/Users/daniel/.rvm/gems/ruby-2.5.1/gems/forest_liana-2.8.2/app/serializers/forest_liana/serializer_factory.rb:11: warning: previous definition of SharedAssetSerializer was here
/Users/daniel/.rvm/gems/ruby-2.5.1/gems/forest_liana-2.8.2/app/serializers/forest_liana/serializer_factory.rb:11: warning: already initialized constant ForestLiana::UserSpace::UserSerializer
/Users/daniel/.rvm/gems/ruby-2.5.1/gems/forest_liana-2.8.2/app/serializers/forest_liana/serializer_factory.rb:11: warning: previous definition of UserSerializer was here```
tvilon commented 6 years ago

I have the same problem

arnaudbesnier commented 6 years ago

Hey @sayduck-daniel, thanks for the report. Did those warning appear since a specific Forest liana version? Or did they appear after the ruby version upgrade on your projects?

Thanks for the help.

ghost commented 6 years ago

I can’t recall if it was during an upgrade of forest or not. I’m using 2.5.1 ruby.

tvilon commented 6 years ago

With forest_liana 2.11.5

For us it happened when we added smart action on the 3rd collection. Originally we had smart actions only on User and Organization and it worked fine.

Then we added the skeleton for a new smart action on UserReview and that generated the warning. I say skeleton because indeed you only need to create the /lib/forest_liana/collections/xxxx.rb to have the warning.

# /lib/forest_liana/collections/user_review.rb
class Forest::UserReview
  include ForestLiana::Collection

  collection :UserReview

end

Findings

The warning fires super early in the rais boot. Even before rails checks if another server is already running.

It looks like ForestLiana::UserSpace::UserReviewSerializer can be defined twice

-https://github.com/ForestAdmin/forest-rails/blob/208b987e3081feea2202fff02e66fa314de611a1/lib/forest_liana/collection.rb#L17 -https://github.com/ForestAdmin/forest-rails/blob/be30d86774c04552059e7c32d1c27452729f4ab4/app/serializers/forest_liana/serializer_factory.rb#L11

melcher commented 6 years ago

Getting the same error with 2.13.7:

/Users/user/gems/forest_liana-2.13.7/app/serializers/forest_liana/serializer_factory.rb:11: warning: already initialized constant ForestLiana::UserSpace::CompanySerializer
/Users/user/gems/forest_liana-2.13.7/app/serializers/forest_liana/serializer_factory.rb:11: warning: previous definition of CompanySerializer was here
/Users/user/gems/forest_liana-2.13.7/app/serializers/forest_liana/serializer_factory.rb:11: warning: already initialized constant ForestLiana::UserSpace::InvoiceSerializer
/Users/user/gems/forest_liana-2.13.7/app/serializers/forest_liana/serializer_factory.rb:11: warning: previous definition of InvoiceSerializer was here
/Users/user/gems/forest_liana-2.13.7/app/serializers/forest_liana/serializer_factory.rb:11: warning: already initialized constant ForestLiana::UserSpace::ReceiptSerializer
/Users/user/gems/forest_liana-2.13.7/app/serializers/forest_liana/serializer_factory.rb:11: warning: previous definition of ReceiptSerializer was here
/Users/user/gems/forest_liana-2.13.7/app/serializers/forest_liana/serializer_factory.rb:11: warning: already initialized constant ForestLiana::UserSpace::PartnerSerializer
/Users/user/gems/forest_liana-2.13.7/app/serializers/forest_liana/serializer_factory.rb:11: warning: previous definition of PartnerSerializer was here

That's just a snippet, we get about 20 of these errors on every rails boot / env load. We treat messages printed at load to be warnings / errors that need to be addressed but have trouble seeing them given the amount of text that now appears on load. Would be great if this could be addressed.

KidA001 commented 6 years ago

I also started getting this error on forest_liana 2.13.6 once I added a custom field

# frozen_string_literal: true

class Forest::Company
  include ForestLiana::Collection

  collection :Company

  field :logo, type: 'String' do
    object.try(:logo).try(:service_url)
  end
end
melcher commented 6 years ago

I think the issue may be when we use collection :ClassName on the collection models that we define. Exactly what KidA001 posted above.

the line collection :Company calls https://github.com/ForestAdmin/forest-rails/blob/9b1531a34bde74de21ce9630002c297b7f562851/lib/forest_liana/collection.rb#L21

This calls .serialize_for here: https://github.com/ForestAdmin/forest-rails/blob/9b1531a34bde74de21ce9630002c297b7f562851/app/serializers/forest_liana/serializer_factory.rb#L56

which calls .define_serializer here: https://github.com/ForestAdmin/forest-rails/blob/9b1531a34bde74de21ce9630002c297b7f562851/app/serializers/forest_liana/serializer_factory.rb#L280

Then, when Forest loads itself through the bootstrapper in #perform it loads all the definitions again here: https://github.com/ForestAdmin/forest-rails/blob/be30d86774c04552059e7c32d1c27452729f4ab4/lib/forest_liana/bootstraper.rb#L75

This re-defines the constant and causes the warning message.

For our codebase, all of the classes with this warning (e.g. User) are actually activerecord models, so the code-path it's following (if smart_collection? &&) is incorrect. In other words, the first load (where it's identified as a "smart_collection?") is incorrect. Not redefining the constant would break, as it would be defined as a 'smart_collection').

KidA001 commented 6 years ago

actually, I'm realizing this is because I have my rails application eagerload my lib directory, as I'm using it for other lib objects in my application application.rb

...
config.eager_load_paths << Rails.root.join('lib')
config.watchable_dirs['lib'] = [:rb]
...

It seems I need to eager-load lib and figure out how to exclude forest_liana dir

ghost commented 6 years ago

Awesome track down and f the issue. It appears I load lib as well.

Let me know if you found a work around to exclude the folder

On 23 Oct 2018, at 2.23, Brian Vogelgesang notifications@github.com wrote:

actually, I'm realizing this is because I have my rails application eagerload my lib directory, as I'm using it for other lib objects in my application application.rb

... config.eager_load_paths << Rails.root.join('lib') config.watchable_dirs['lib'] = [:rb] ... It seems I need to eager-load lib and figure out how to exclude forest_liana dir

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

arnaudbesnier commented 5 years ago

Fixed in v4.0.1 @sayduck-daniel ?

ghost commented 5 years ago

Damn I forgot about this issue... Yes indeed, it's fixed