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

Update getProductOptions to handle divergent options #2747

Merged
merged 6 commits into from
Feb 12, 2025

Conversation

wizardlyhel
Copy link
Contributor

WHY are these changes introduced?

getProductOptions will fail on combined listing products with divergent options. For example, we have a parent product TV and the only common option between its child products is size of the TV.

getProductOptions fails at finding a matching encoding and mapping with the selected option.

WHAT is this pull request doing?

  • Update the variant mapping so that we are not reliant on the encoding
  • Update the encoding matching so that it doesn't fail on optimistic variant selection on non-existing option value

HOW to test your changes?

Post-merge steps

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or noteworthy changes
  • I've added tests to cover my changes
  • I've added or updated the documentation

Copy link
Contributor

shopify bot commented Feb 10, 2025

Oxygen deployed a preview of your hl-fix-getProductOption branch. Details:

Storefront Status Preview link Deployment details Last update (UTC)
custom-cart-method ✅ Successful (Logs) Preview deployment Inspect deployment February 11, 2025 5:28 PM
metaobjects ✅ Successful (Logs) Preview deployment Inspect deployment February 11, 2025 5:28 PM
classic-remix ✅ Successful (Logs) Preview deployment Inspect deployment February 11, 2025 5:28 PM
third-party-queries-caching ✅ Successful (Logs) Preview deployment Inspect deployment February 11, 2025 5:28 PM
Skeleton (skeleton.hydrogen.shop) ✅ Successful (Logs) Preview deployment Inspect deployment February 11, 2025 5:28 PM

Learn more about Hydrogen's GitHub integration.

Comment on lines 126 to 128
return Object.assign(
{},
...selectedOption.map((option) => ({[option.name]: option.value})),
Copy link
Contributor

Choose a reason for hiding this comment

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

This is now going to return an Object rather than a string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh oops! nice catch

Copy link
Contributor

@juanpprieto juanpprieto left a comment

Choose a reason for hiding this comment

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

thanks LGTM!

@wizardlyhel wizardlyhel merged commit 3af2e45 into main Feb 12, 2025
12 checks passed
@wizardlyhel wizardlyhel deleted the hl-fix-getProductOption branch February 12, 2025 17:25
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