gatsbyjs / gatsby-starter-shopify

A Gatsby starter using the latest Shopify plugin showcasing a store with product overview, individual product pages, and a cart
https://shopify-demo.gatsbyjs.com/
BSD Zero Clause License
311 stars 137 forks source link

Add SSR support to search page (G4) #50

Closed jxnblk closed 3 years ago

jxnblk commented 3 years ago

This change adds SSR to the /search page using the Gatsby 4 beta release. The server response uses the URL query parameters to fetch the filtered product list for the HTML response.

gatsby-cloud[bot] commented 3 years ago

Gatsby Cloud Build Report

gatsby-starter-shopify

:tada: Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

:clock1: Build time: 11m

Performance

Lighthouse report

Metric Score
Performance :large_orange_diamond: 85
Accessibility :green_heart: 100
Best Practices :green_heart: 93
SEO :green_heart: 92

:link: View full report

aghreed commented 3 years ago

So I used this review as an opportunity to finally set up a shopify development store and get the starter running locally.

I am currently not able to run yarn build/gatsby build successfully: Screen Shot 2021-10-13 at 12 29 15 PM

I could just be missing a setup step, but wanting to flag it regardless.

On another note, npm i does not work on this branch, but yarn does. I bring this up because there's not a mention of any dependencies install within the README and could see this being a point of confusion for some people, especially when you run gatsby develop (the first instruction in the README), you get this message back:

There was a problem loading the local develop command. Gatsby may not be installed in your site's "node_modules" directory. Perhaps you need to run
"npm install"? You might need to delete your "package-lock.json" as well.

FWIW, the feedback after an npm install:

Screen Shot 2021-10-13 at 2 26 48 PM
jxnblk commented 3 years ago

@aghreed Can confirm that I'm seeing similar errors using npm i – this looks like the same peerDependencies issue we ran into with other prerelease versions. You can use npm i --force while we're in beta, but we should follow up and ensure these all work with the stable release once that's out

jxnblk commented 3 years ago

It's probably also worth updating the instructions in the README to use npm start instead of assuming a user has gatsby installed globally, which they might not if they use npx gatsby new

aghreed commented 3 years ago

It's probably also worth updating the instructions in the README to use npm start instead of assuming a user has gatsby installed globally, which they might not if they use npx gatsby new

Yeah I think that would be a nice addition. I also had a bit of an issue piecing together all the steps needed to get this up and running locally, specifically around creating the .env and plugging in my values. It's mentioned here but if you're walking through that README step by step, you're going to experience issues before getting that far down the README.