-
Notifications
You must be signed in to change notification settings - Fork 686
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
[PWA-2498] Rename Link's prefetchType prop #3646
Conversation
|
@magento create issue from PR |
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.
@jimbo
Code looks good except single comment about TODO. I added a minor version label + fixed the "closes issue" in description. Warning should be gone next checks run.
*/ | ||
const Link = props => { | ||
const talonProps = useLink(props); | ||
// TODO: remove `prefetchType` |
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.
Should this TODO be removed?
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.
No, the prop still exists so as not to be a breaking change. We can actually remove it in a future major release, and delete the comment at that time.
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.
Code looks good. Approved.
QA has passed for this ticket and will be merged momentarily. Thanks! |
Description
For the
Link
component, rename the boolean propprefetchType
toshouldPrefetch
. The former appears to indicate it will contain a type rather than a boolean, causing some confusion. The approach hereinprefetchType
prop toLink
anduseLink
, but does not remove itshouldPrefetch
as a new prop to both component and talonprefetchType
unless it istrue
Related Issue
Closes #3647
Acceptance
Verification Stakeholders
Specification
Verification Steps
Links with either value of
prefetchType
should work the same as they do now. That said, we don't appear to have any.Test scenario(s) for direct fix/feature
Test scenario(s) for any existing impacted features/areas
Test scenario(s) for any Magento Backend Supported Configurations
Is Browser/Device testing needed?
Any ad-hoc/edge case scenarios that need to be considered?
Screenshots / Screen Captures (if appropriate)
Breaking Changes (if any)
These changes should be non-breaking. Eventually, we should remove the deprecated prop, which would be a breaking change.
Checklist
Resolved issues: