thetrevorharmon / gatsby-theme-shopify-manager

The easiest way to start a Shopify shop on Gatsby.
https://gatsby-theme-shopify-manager.netlify.app/
MIT License
121 stars 11 forks source link

Decoupling `gatsby-source-shopify` #55

Closed beamercola closed 3 years ago

beamercola commented 4 years ago

Hey Trevor 👋🏼 we chatted at Gatsby Days in LA. This is everything I've wanted - so thank you.

I'm running a custom gatsby-source-shopify. i wonder if makes sense to decouple your library from the source. It's cool you proxy the settings down to it, but it seems to be in the name of not adding 2 libraries.

Curious to hear your thoughts on it.

thetrevorharmon commented 4 years ago

Hey man! So you can actually prevent it from including the source plugin by passing shouldIncludeSourcePlugin: false in the options. Would what you’re describing be accomplished with that setting?

Docs link: https://gatsbythemeshopifymanager.com/#shouldincludesourceplugin

beamercola commented 4 years ago

Cool! I must have glazed right past that. Yes that's exactly what I want - continue as you were

thanks

beamercola commented 4 years ago

Not sure if this is related but I'm getting an error on deploy once i did that

3:57:43 PM: npm
3:57:43 PM: ERR! code ELIFECYCLE
3:57:43 PM: Failed during stage 'building site': Build script returned non-zero exit code: 1
3:57:43 PM: npm
3:57:43 PM: ERR! errno 1
3:57:43 PM: npm
3:57:43 PM:  ERR! gatsby-source-shopify@3.2.0 build: `babel src --out-dir . --ignore "**/__tests__"`
3:57:43 PM: npm ERR! Exit status 1
3:57:43 PM: npm ERR!
3:57:43 PM: npm ERR! Failed at the gatsby-source-shopify@3.2.0 build script.
3:57:43 PM: npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
3:57:43 PM: npm ERR! A complete log of this run can be found in:

In my lockfile I see both:

"gatsby-source-shopify@https://github.com/beamercola/gatsby-source-shopify.git":
  version "3.2.0"

gatsby-source-shopify@^3.0.43:
  version "3.2.8"

Am I doing something wrong here?

thetrevorharmon commented 4 years ago

Ah now I understand. You're using a forked version of gatsby-source-shopify. Gatsby-theme-shopify-manager still installs its own version of gatsby-source-shopify even if you pass shouldIncludeSourcePlugin–that plugin only omits the config object in the gatsby-config file. I wonder if this needs to be refactored to not install the source plugin for you.

thetrevorharmon commented 4 years ago

Do you know if you can conditionally require/import/install node packages in a package.json? I assume you can't, but maybe you know better than I do

beamercola commented 4 years ago

can you set it in optionalDependencies? Not sure of the ramifications of that.

Is there an easy way to fork this and comment something out for a quick fix? Or is it more embedded in there

beamercola commented 4 years ago

Also, i know i'm a bit of an edge case. A little backstory: https://github.com/gatsbyjs/gatsby/pull/23840#issuecomment-629852784

thetrevorharmon commented 4 years ago

Yeah, I remember you reaching out about the images challenge that you were facing, and so I figured that you had a forked version for that reason. And even if it's an edge case, I think that it makes sense to consider (the whole reason why the options even exist are for edge cases, haha).

I think the steps we need to take are this:

Would this be something you would be able to help with? Fork the repo, make the changes, and open a PR?

beamercola commented 4 years ago

aye aye cap'n - on it

beamercola commented 4 years ago

Pull Request: #57

thetrevorharmon commented 4 years ago

@beamercola did you see my changes request on your PR?

thetrevorharmon commented 3 years ago

@beamercola thanks for your contribution! With the merging of https://github.com/thetrevorharmon/gatsby-theme-shopify-manager/pull/77, the source plugin is now decoupled.