Shopify / hydrogen

Hydrogen lets you build faster headless storefronts in less time, on Shopify.
https://hydrogen.shop
MIT License
1.21k stars 243 forks source link

Add codegen to demo-store #937

Closed frandiox closed 1 year ago

frandiox commented 1 year ago

Related #887 Closes #939

This PR adds generated types to most of the files of the the demo-store. There are still 2 or 3 remaining that require heavier changes.

I've fixed some bugs while doing this, mostly due to missing fields in queries that were supposed to be used later for SEO or related.

Questions:

github-actions[bot] commented 1 year ago

We detected some changes in packages/*/package.json or packages/*/src, and there are no updates in the .changeset. If the changes are user-facing and should cause a version bump, run npm run changeset add to track your changes and include them in the next release CHANGELOG. If you are making simple updates to examples or documentation, you do not need to add a changeset.

blittle commented 1 year ago

If there's an edges property, can we always assume there will be an edges.node property?

frehner commented 1 year ago

If there's an edges property, can we always assume there will be an edges.node property?

Not necessarily. Someone could query

collections() {
  edges {
    cursor
   }
}

hypothetically. https://shopify.dev/docs/api/storefront/2023-01/objects/CollectionEdge 🤷

frehner commented 1 year ago

It looks like flattenConnection has a problem with some of the generated types. Some times, the generated type has {edges: {node: Array}} but flattenConnection expects {edges: {node?: Array}}, with that optional ?. @frehner any idea how to fix this? 🤔

Can you point me to an example of where this issue is? Thanks!

frandiox commented 1 year ago

Can you point me to an example of where this issue is? Thanks!

Sure, right here: https://github.com/Shopify/hydrogen/blob/acd206bdbf29ea8775554f9727f1fdd2ac5bd695/templates/demo-store/app/routes/(%24locale).journal._index.tsx#L29

There are a few // TODO like this in the PR but this is the one I was trying to fix and sadly couldn't.

Perhaps related to using (first: $pageBy, after: $cursor) in the query? 🤔

frehner commented 1 year ago

@frandiox thanks! I put up a PR to fix things in flattenConnection() https://github.com/Shopify/hydrogen/pull/945

blittle commented 1 year ago

For changelogs... would this become demo-store@2.0.0?

I'm not sure how to version the demostore. What is a breaking change when anything we add requires the merchant to update their store if they want the applied changes? I'd be fine with leaving it as minor, unless someone else has a strong opinion

frehner commented 1 year ago

flattenConnection changes have been merged, so you should be able to update from upstream and get those fixed.