Shopify / shopify-api-ruby

ShopifyAPI is a lightweight gem for accessing the Shopify admin REST and GraphQL web services.
MIT License
1.06k stars 473 forks source link

Make logger untyped #1226

Closed vinistock closed 1 year ago

vinistock commented 1 year ago

Description

In this commit which is part of Rails 7.1, the default logger changed from Logger to the new BroadcastLogger.

This change causes a sorbet-runtime error because BroadcastLogger does not inherit from Logger and therefore doesn't match the expected type in the signature for the setup method.

This ends up being a weird issue. The BroadcastLogger class was designed to match the same interface as Logger. However, we can't use a Sorbet interface in this case. For an interface to work, it needs to be included in the actual runtime - and Rails won't include a Sorbet construct in the codebase.

Even using T.any(Logger, Rails::BroadcastLogger) wouldn't work properly. This gem has a dependency on Active Support, but using the new BroadcastLogger would create a dependency on Active Support >= 7.1.0, which is probably too new for all consumers of this gem.

Therefore, despite the unfortunate loss in type safety, I think the way forward is to set logger to T.untyped so that we can accept both Logger and BroadcastLogger without creating a dependency on Rails.

This unblocks Rails 7.1 upgrades.

How has this been tested?

Just ran bundle exec rake test.

Checklist: