1834 link component is not checking if href is undefined
- [x] This PR follows the Contribution Guide
- [x] Changelog updated
- [ ] Upgrade guide entry added
Description / Motivation
Describe the Bug If the field passed to the Link component has an undefined href, it will render undefined as a string in the dom.
import {Link} from '@sitecore-jss/sitecore-jss-nextjs';
const field = {value: {href: undefined, text: 'This is a link with a href that is undefined'}};
<Link field={field} />
This will render a link like this:
<a href="undefined">This is a link with a href that is undefined</a>
Expected Behavior
<a>This is a link with a href that is undefined</a>
Testing Details
- [x] Unit Test Added
- [ ] Manual Test/Other (Please elaborate)
Types of changes
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
Thanks for tackling this @sumon1991 🙏🏼
I noticed that there are tests that pass in undefined href values and then it's supposed to render nothing:
- https://github.com/Sitecore/jss/blob/dev/packages/sitecore-jss-nextjs/src/components/Link.test.tsx#L508-L536
- https://github.com/Sitecore/jss/blob/dev/packages/sitecore-jss-react/src/components/Link.test.tsx#L222-L267
Now wondering why they are not failing and/or why the component still rendered with the undefined string value 😕
Probably also worth adding a test to check the fix this PR adds.
@pzi do I need to update the test cases?
@pzi do I need to update the test cases?
Ultimately, it's up to the Sitecore folks to decide that, but I, personally, would add one so it captures that decision to specifically render links without href attributes. Also means it will highlight any regressions if it accidentally gets changed in the future.
After some investigation: the link.href is already being verified in here: https://github.com/Sitecore/jss/blob/85ec232ed9a7928c6066f3abbc03f80d1f89afcb/packages/sitecore-jss-react/src/components/Link.tsx#L50
This is why unit tests are not failing, if href is not there, we consider the link field empty. I believe that change went live around the same time you posted the issue and the PR @sumon1991 , as a part of unrelated feature.
I'll close the PR and the related issue, as, in the end, the issue should no longer be present - but please do reopen the issue and the PR if you still encounter the problem.