-
Notifications
You must be signed in to change notification settings - Fork 122
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
@W-14901147: Shopper SEO hook #1688
Conversation
packages/commerce-sdk-react/src/hooks/ShopperSeo/queryKeyHelpers.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 from me. Had a question though about showing example usage.
And as an aside: maybe it's just me.. I found the code not really readable 😅 Maybe it's because of the typing. Perhaps the library is also really doing complex things.. 🤷 But anyways, other files are already coded in a similar way.
//TODO: Remove after PR is approved. This is for hook testing and verifying | ||
const {data} = useUrlMapping({ | ||
parameters: { | ||
urlSegment: '/sample-product' | ||
} | ||
}) | ||
console.log('data', data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After this test code is removed, will the retail-react-app show an example of how to really use useUrlMapping
?
For example, right now I don't know whether I'll need to update my routes. Where should I call useUrlMapping
?
I think it would be good if we can leave commented code somewhere, as a best practice on how to use this url mapping feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vmarta I think useUrlMapping is like any other query hooks in the library. You can certainly call it any where you want in the app as long as you know which parameters to pass in the hook. (we currently do not have any examples for unused hooks in commerce-react)
In addition, we will start the Support ShopperSEO url mapping work soon, I think we will have more context to document there.
Description
This PR added query hooks for Shopper SEO APIs that has been added into
commerce-sdk-isomorphic
v1.12.0Types of Changes
Changes
How to Test-Drive This PR
For testing purpose of this API, I've added redirects in BM that will used in the template Home page.
Source: /sample-product
Destination: /sample-product-redirected
npm start
to start up Retail appChecklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization