Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

context.js updates w/ graphQL #129

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

jpvalery
Copy link

@jpvalery jpvalery commented Nov 2, 2023

(Tentative for #125)

@ndimatteo this is how far I managed to get but I'm scratching my head with SiteContextProvider :(

In this current state, the project builds and deploys. But products are stuck on "loading" (see my branch preview).

I think that there's a tiny bit missing in SiteContextProvider to get this to work. I have tested all the queries and they're working.

Any insights or guidance you could provide to help me finish shipping this would be greatly appreciated 🙏

Copy link

vercel bot commented Nov 2, 2023

Someone is attempting to deploy a commit to a Personal Account owned by @ndimatteo on Vercel.

@ndimatteo first needs to authorize it.

@ndimatteo
Copy link
Owner

Hey there @jpvalery, I appreciate the work you put into this! I checked your branch preview, but I'm not seeing any console errors (aside from some vercel.live api calls).

It's a bit hard for me to decipher what's going on in your PR, both in terms of switching things to cart creation with graphQL as opposed to checkout creation with the buy sdk, as well as the formatting for the file being incorrect (looks like you didn't have prettier set up to match the project settings?).

I truly do not have the capacity to dig into this further right now, and like I mentioned before– I have plans to switch to hydrogen-react for managing all the cart interactions, which will remove the SiteContextProvider feature from HULL completely.

I hope you can understand, and if you are able to debug this on your own (and get the formatting fixed up), I'm happy to test and merge!

lib/context.js Outdated Show resolved Hide resolved
@jpvalery
Copy link
Author

jpvalery commented Nov 2, 2023

@ndimatteo you're welcome! Totally understand the lack of bandwidth 👍

Apologies for the prettier mixup. I've updated that branch to use your config.

I'll keep hacking at this in the hope I can fix it. Worst case scenario, the queries above might help your migration.

@jpvalery
Copy link
Author

jpvalery commented Nov 3, 2023

So after a lot of head-scratching, I started from scratch (no pun intended). I got something working pretty flawlessly BUT, I don't know if it should be merged as is.

  1. I've left most of the existing code for reference and commented it out where possible.
  2. I couldn't get the setContext to work as I wanted so I did quite a few workarounds that work for me—but unsure if you wanna use that approach yourself
  3. On the Cart, I chose speed over quality and replaced the <Photo> by an <img> with the picture coming from Shopify directly

These caveats being outlined, this works: products (including bundles) can be added/updated/removed. The cart persists.
CleanShot 2023-11-02 at 22 32 23

I'll leave that PR and branch available should anyone face the same issue ✌️

I appreciate your availability and feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants