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

rerender seems to update props instead of replacing them #252

Closed
MysticEarth opened this issue Oct 13, 2021 · 3 comments · Fixed by #257
Closed

rerender seems to update props instead of replacing them #252

MysticEarth opened this issue Oct 13, 2021 · 3 comments · Fixed by #257

Comments

@MysticEarth
Copy link

MysticEarth commented Oct 13, 2021

In a component we provide two inputs, avatar and name. Whenever avatar is undefined and a name is given, it should render the initials instead of an image. To test this behaviour we've used rerender:

rerender({ avatar: 'https://clipground.com/images/img_avatar-png-2.png' });

const renderedAvatar = screen.getByRole('img');

expect(renderedAvatar).toBeInTheDocument();
expect(renderedAvatar).toHaveAttribute('src', 'https://clipground.com/images/img_avatar-png-2.png');

...

rerender({ name: 'John Appleseed' });

// Initials should only exist if `avatar` is `undefined`
const renderedInitials = screen.getByText(/J/); 

(renderedInitials).toBeInTheDocument();

renderedAvatar succeeds as expected, but renderedInitials fails since avatar is still set – while I expected passing an object to rerender would completely replace all props.

The test succeeds when I explicitly set avatar:

rerender({ avatar: undefined, name: 'John Appleseed' });

// Initials should only exist if `avatar` is `undefined`
const renderedInitials = screen.getByText(/J/); 

(renderedInitials).toBeInTheDocument();
@timdeschryver
Copy link
Member

timdeschryver commented Oct 16, 2021

Thanks for opening this issue @MysticEarth.
It seems like the other TL libraries are expecting like this, so we'll have to change our behavior.

The reason why it behaves like this, is to be able to test ngOnChange - which I think is valuable,
Because of this, I think the current render method should be renamed to change or update or something like that...

It would require a breaking change, so it could be a while before this lands.
Maybe with Angular v13...

@MysticEarth
Copy link
Author

Ah yes, I understand! Sounds great you are introducing a new change method for this purpose.

@github-actions
Copy link

🎉 This issue has been resolved in version 11.0.0-beta.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants