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

Optimize query for product page #918

Merged
merged 6 commits into from
Mar 15, 2022
Merged

Conversation

michenly
Copy link
Contributor

@michenly michenly commented Mar 15, 2022

Description

Part of Shopify/hydrogen#778

What was done:

  • remove most of the hinted fields that was potentially over fetching. (But kept video related fields to account for the variety of all the media types even though there is no videos media file in the demo shop.)
  • inline all query variables that is not passed into useShopQuery
  • order most of the fields and remove duplicated fields

@michenly michenly requested review from frehner and jplhomer March 15, 2022 17:20
Copy link
Contributor

@frehner frehner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think line 62 has a duplicate id that could be removed.

Also, do you mind removing as many variables as you can as well? Since these queries aren't dynamic anymore, there's no point in having those variables there I don't think.

@michenly
Copy link
Contributor Author

@frehner so inline all the variables as well?

Copy link
Contributor

@jplhomer jplhomer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to removing variables, where did we land on rewriting the metafields to the new specific metafield version?

@michenly
Copy link
Contributor Author

@jplhomer , @frehner mentioned that we are keeping the deprecated metafield fields till 2022-07 API version

@frehner
Copy link
Contributor

frehner commented Mar 15, 2022

@frehner so inline all the variables as well?

As many as you can. I think the country variable should remain, but most everything else can be inlined. See https://github.com/Shopify/hydrogen/pull/919/files as a reference if that helps.

Copy link
Contributor

@frehner frehner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Awesome work, thank you!

@michenly michenly merged commit 5699e0e into v1.x-2022-07 Mar 15, 2022
@michenly michenly deleted the optimize-product-query branch March 15, 2022 18:05
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.

3 participants