5.2.0 breaks some custom loggers

[tragiclifestories]

We have our own internal logging library that enforces our company standards. It implements #tagged identically to ActiveSupport's TaggedLogging.

Before #1311, webpacker would happily use our custom logger. But that PR flips the if statement, and now Webpacker is messing with our logger, replacing our formatter with some random unwanted one which is incompatible with our gem (we only emit structured JSON logs and handle the lines internally as hashes, so we get no implicit conversion errors instead of log lines).

[gauravtiwari]

@tragiclifestories sorry about that - lets revert that change 👍

[tragiclifestories]

Cheers! I think there must be a way to fix the original reported problem without breaking us, but I can't see one just yet.

[gauravtiwari]

No worries, released 5.2.1

Yes, me neither. Makes sense to revert since it's possible a lot of people might be customising the logger.

// @flippakitten

[gauravtiwari]

Feel free to close if all good

[tragiclifestories]

Looks good. Thanks for the quick fix!

[flippakitten]

Wow, this PR is a blast from the past.

If my memory serves me correctly, the issue was around centralized logging and when you completely replace the rails logger so when Webpacker initializes it overwrites your logger with a new TaggedLogging instance.

@tragiclifestories Sorry about introducing breaking changes.

@gauravtiwari what about a config option or revert to the current implementation?

  initializer "webpacker.logger" do
    config.after_initialize do
      if ::Rails.logger.respond_to?(:tagged) || use_default_rails_logger
        Webpacker.logger = ::Rails.logger
      else
        Webpacker.logger = ActiveSupport::TaggedLogging.new(::Rails.logger)
      end
    end
  end

EDIT: looking a bit deeper, I do admit my fix was not the best solution.

[scottrobertson]

What changes happened in 5.2.0? No change log entries.

[gauravtiwari]

@flippakitten thank you for update. Is it not possible to override Webpacker.logger in the app itself?

@scottrobertson Mainly, dependencies update and some minor fixes: v5.1.1...5-x-stable

[emilkarl]

Reverting this actually breaks it for us when using lograge. Changing it to the other way around works tho. Any idea why?

Edit: Using webpacker 5.2.1 on Rails 5.2.4.4

Error

NoMethodError: undefined method `formatter=' for #<Logging::Logger:0x00007fc0c0edcb18>
web          | Did you mean?  formatter
web          |   /Users/emil/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/activesupport-5.2.4.4/lib/active_support/tagged_logging.rb:63:in `new'
web          |   /Users/emil/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/webpacker-5.2.1/lib/webpacker/railtie.rb:34:in `block (2 levels) in <class:Engine>'

This change make it work 🤷‍♂️

initializer "webpacker.logger" do
  config.after_initialize do
    if ::Rails.logger.respond_to?(:tagged)
      Webpacker.logger = ActiveSupport::TaggedLogging.new(::Rails.logger)
    else
      Webpacker.logger = ::Rails.logger
    end
  end
end