solidusio / solidus

🛒 Solidus, the open-source eCommerce framework for industry trailblazers.
https://solidus.io
Other
5.02k stars 1.29k forks source link

[RFC] Move Paperclip into an extension #2324

Closed swcraig closed 4 years ago

swcraig commented 7 years ago

Since the earliest releases of the project, image uploading and display was handled by has_attached_file and Paperclip.

Paperclip was the first and most widely adopted file upload gem in Rails, and has done an excellent job in its 10 year history - both on its own and for the Spree and Solidus projects.

However, as web apps have evolved so have our file/image management needs and options. Not all stores use Paperclip (or Spree::Image) to store or manage its image assets. Many stores hack in alternative options like imgix, Contentful, Alchemy, or a host of others to handle image management and display.

As far as Thoughtbot is concerned, Paperclip is in a semi-official state of non-maintenance.

Tying Solidus to a single file upload library, and one that is very likely to be unmaintained in the near future, does not seem to be an ideal situation going forward. There are alternatives to Paperclip, including ActiveStorage in Rails 5.2, and we should allow as much room as possible for people to use any of these libraries in Solidus. None of these libraries have identical feature sets, and forcing Solidus users to use any single one of these is restrictive. Pulling Paperclip out of Solidus into an extension makes room for custom implementations, and also gets rid of the ImageMagick dependency for installs.

I believe the core of Solidus needs to start extracting the Paperclip functionality into a gem, and provide a better interface for alternative image implementations.

There isn’t an easy or clear path to extract Paperclip with minimal disruption to existing stores, but I believe the path laid down here is the best starting point. It follows the pattern Clarke attempted in #625, but in a much lighterweight and gem-dependent manner.

If we can adopt the idea of a “gallery” on Spree::Variant and Spree::Product, we can rely on that in core and push implementation of image handling to a suite of extensions (providing solidus_paperclip to start, and by default). Starting off this way will also allow us to move forward for now without changing any of the views of Solidus - they mostly act like they do when a variant/product doesn't have images.

If we were to accept the idea of a "gallery", this proposed PR would remove all Paperclip specific code from Solidus. It is still very much a work in progress.

I'm happy to have a discussion about this path here, or feel free to ping me (@swcraig) on the Solidus slack

cbrunsdon commented 7 years ago

Full disclosure I directed and helped @swcraig so I'm implicitly biased but also :+1:.

We often don't use paperclip in all of stores, and where we do use it we (maybe) probably shouldn't.

Pulling it out into an extension and preparing for the future or alternative image implementations is a :+1: for me, as long as you can drop in the gem seamlessly.

jhawthorn commented 7 years ago

I'd be happy as long as we can ship an image provider to new users relatively seamlessly.

If we can get this so we just change our README to recommend

gem 'solidus'
gem 'solidus_auth_devise'
gem 'solidus_paperclip'

as the starting Gemfile and provide the same experience we have now, that would be :+1:

jhawthorn commented 7 years ago

We also need to consider how to cleanly extract paperclip from Taxon, which has an icon (although I'm not sure it's used anywhere).

It's probably worth asking in slack to see how widely this is used.

swcraig commented 4 years ago

I think general consensus is that we should move off of Paperclip.

There is ongoing work to add ActiveStorage support in https://github.com/solidusio/solidus/pull/2974.