gatsbyjs / gatsby

The best React-based framework with performance, scalability and security built in.
https://www.gatsbyjs.com
MIT License
55.27k stars 10.31k forks source link

Add option to disable downloading of images when using the `gatsby-source-shopify` plugin. #23724

Closed lukebennett88 closed 3 years ago

lukebennett88 commented 4 years ago

Summary

I'm working on rebuilding an existing Shopify site as a Gatsby site using the gatsby-source-shopify plugin. It has ~600x items, and my internet connection isn't particularly good, so it can take a very long time for the site the build (even in develop mode). Sometimes the build will fail completely. If sharp tries to transform any of the images it takes even longer and is even less stable.

I think it might be a good idea to add an option to disable downloading the images, as we can still serve the images from Shopify's CDN. The Shopify CDN will let you resize images using the URL, so the performance is still pretty good. Here is a page with some info on doing this: https://www.shopify.com/partners/blog/img-url-filter

Basic example

plugins: [
  {
    resolve: `gatsby-source-shopify`,
    options: {
      shopName: process.env.SHOPIFY_SHOP_NAME,
      accessToken: accessToken: process.env.SHOPIFY_ACCESS_TOKEN,

      // Whether or not images should be downloaded locally so that they can be
      // transformed by sharp. You might want to disable this for stores with a
      // lot of products
      // Defaults to true
      downloadImage: false,
    },
  },
];

Motivation

Adding this option will make it much easier for people to use this plugin if they have poor internet connections, less powerful computers, or are using it to connect to a store with a large number of images to download.

I plan to attempt to add this myself (along with the help of my friend @mrhut10), but I've never written/worked on a Gatsby plugin before, and have limited knowledge of GraphQL outside of using it within Gatsby.

I also wanted to make sure it sounded like a good idea first, and that I'm not overlooking anything before doing the work and submitting a PR only to have it rejected.

If this goes well, I want to also work on a companion plugin to use the images from the Shopify CDN in gatsby-image in much the same way as gatsby-transformer-cloudinary

I'm not sure if this is possible since we need a base64 image as well, but I'll open a seperate issue for that once I get some feedback on this issue.

LekoArts commented 4 years ago

Thank you for opening this!

Can you give us some more information on your issue? You should already be able to not use sharp by not querying localFile: https://www.gatsbyjs.org/packages/gatsby-source-shopify/#image-processing

Moreover, you could also limit your GraphQL query to not query 600 items but e.g. 50 if your current internet is too slow. There are also issues open like https://github.com/gatsbyjs/gatsby/issues/10049 and https://github.com/gatsbyjs/gatsby/issues/13909 that want to improve this even further.

So I'm not sure if it's necessary to add another option to this specific plugin.

lukebennett88 commented 4 years ago

Hi @LekoArts,

You're quite right that not referencing localFile stops sharp from transforming every image, but it doesn't stop the images being downloaded. It seems unnecessary for someone to download every file if they only plan to use the CDN link.

LekoArts commented 4 years ago

Ah, gotcha. So it would make sense to include that option then.

lukebennett88 commented 4 years ago

Sorry if I wasn't clear enough the first time 😊

mrhut10 commented 4 years ago

Yeah from how @lukebennett88 description to me privately if you've got 500+ products with 5 images on each products then that's 2500 images to download them if you try to use them it will then do all the required transformations. This is huge unneeded downloads, storage and processing when Shopify provides a CDN.

The Gatsby-source-sanity plugin I noticed uses the sanity CDN (or optionally cloudinary)

Hope that helps.

@lukebennett88 and I would loved to help make an attempt at a pr to achieve. But certainly open to help.

mrhut10 commented 4 years ago

I've got a crude implimentation, @LekoArts are @lukebennett88 and I able to book a pair programming sesh to finish it?

open to whoever is best / familiar with the plugin but I have had one pair sesh a long time ago with @pieh which I enjoyed.

regards @mrhut10

github-actions[bot] commented 4 years ago

Hiya!

This issue has gone quiet. Spooky quiet. πŸ‘»

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open! As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! πŸ’ͺπŸ’œ

mrhut10 commented 4 years ago

@lukebennett88 as the creator are you able to tag this as not stale?

we have a PR addressing this issue and some community support.

please let the bots know this issue isn't stale.

@beamercola and @haven3 ??

regards

lukebennett88 commented 4 years ago

Looks like I can @mrhut10. just removed it.

mrhut10 commented 4 years ago

Looks like I can @mrhut10. just removed it.

Looks like you need to yeah tag "not stale" to prevent it automatically being closed by there bots

GooBall commented 4 years ago

Gosh do we need this! I've been running a local fork of gatsby-source-shopify for a while now (doing a similar thing to #23840). We also really need the CDN options. I'm having to use the Shopify Buy SDK to grab the images out the CDN and it is just a bit of a faff.

mrhut10 commented 4 years ago

@gooball I would love to see your implementation incase yours is better than mine? and yes I've been meaning to look at making a gatsby-image compatible shopify cdn component, but haven't had time to look again at this.

otherwise, keep up the conversation hopefully will get reviewed by a core team soon.

mrhut10 commented 3 years ago

@GooBall I would love to see your implementation incase yours is better than mine? and yes I've been meaning to look at making a gatsby-image compatible shopify cdn component, but haven't had time to look again at this.

otherwise, keep up the conversation hopefully will get reviewed by a core team soon.

we are getting close to having this feature merged in, are you still using the shopify sdk?

GooBall commented 3 years ago

@mrhut10 I am still using the sdk in production with my fix to disable the image download.

I actually did an implementation to return the shopify cdn image urls through the gatsby-source-shopify plugin. It worked great and there was a big improvement to site performance (by not having to go through the sdk for images).

But when I deployed it to the production site we ran into cache issues where products had the wrong image showing. I'm not convinced the issue was to do with my implementation and more to do with how the products were mapped over. However, I simply haven't had enough time to revisit it yet.

Congrats on getting the PR merged!

mrhut10 commented 3 years ago

@GooBall is your implementation still up on your github? I'd loved to have a look if your open to it. as would be a killer feature to have, and I'd be interested to see if could reproduce the image cache problem you might have had.

did you write a custom resolver so that you could specify what size image you want in the graphQL query itself to get from shopify?