Discuss desired behavior of rerender

[shaman-apprentice]

Current behavior of rerender

It destroys old component and creates a new one from scratch with given properties.

My thoughts on it

• Name rerender does not suggest to me, that component is created from scratch nor old one is destroyed.
• Angular-testing-library should behave similar to react-testing-library. React's version of rerender does keep existing instance, meaning constructor of class and componentDidMount will not be called. render and componentDidUpdate will get called. Therefore I think rerender should not create the component from scratch but call ngOnChanges with simpleChange parameter containing previous values. Not passed in properties should be removed.
• rerender seems to update props instead of replacing them #252 should work with rerender

When the component in rerender is not created from scratch, I think change is not useful anymore. All properties are controlled from outside. So we can pass in all properties we want to keep in rerender unmodified again. I guess that's why React's API doesn't have an equivalent of change.

[timdeschryver]

Do you want to create a PR for this change @shaman-apprentice ?
I do agree that it should behave the same as the other testing libraries.

[dzonatan]

I know that I'm pretty late to this party but this is change seems like a stepback.
Had a little nightmare to update from v13 to v14 mainly because there is simply no way to actually rerender the component from scratch in a single test case, you're now forced to split it to multiple test cases.
This was pretty handy when you had some logic inside component initialization.
Also, https://testing-library.com/docs/angular-testing-library/api#rerender is outdated in that regard.
But not sure about the "improvement" though. At this point it would be kind of silly to revert this back and make a breaking change once again.

The way componentProperties and componentInputs works now is also pretty confusing and less flexible because of that inputs/properties update logic (<...>Input properties that are not defined are cleared<...>). This is not that big of a deal with smaller components as usually you can pass the same values over and over again but it gets pretty ugly with bigger components that take more complicated objects for their inputs (like form elements, arrays, etc.). Passing these inputs all over again is really not DRY.
This could be fixed by introducing a new property called partialUpdate (or something like that) next to the componentInputs/componentProperties/... so it would be possible to update just one property as it was with the old change utility:

  await rerender({ componentInputs: { firstName: 'John', lastName: 'Smith' } });
  // result: John Smith

   await rerender({ componentInputs: { lastName: 'Smith' } });
  // result: Smith

  await rerender({ componentInputs: { firstName: 'Jacob' }, partialUpdate: true });
  // result: Jacob Smith

[shaman-apprentice]

Thanks for joining the party and your input :)

Following just my quick evening thoughts:

but this is change seems like a stepback.

I still think this was a good idea. My two main selling points are still:

• testing-library is an abstracted approach. Adopting your testing-library/react knowledge to testing-library/angular, testing-libary/... should be obvious. I guess testing-library/react rerender is the guideline with most followers / the reference implementation?
• rerender is a bad name for something which creates something from scratch when there is a life cycle (whether it would be better to always have a declarative approach where the same inputs always leads to the same result is out of scope here from my point of view :) )

Also, https://testing-library.com/docs/angular-testing-library/api#rerender is outdated in that regard.

Good catch! Would you mind to create a PR to update it? If you don't have time, I can do that too.

[...] there is simply no way to actually rerender the component from scratch in a single test case

I haven't checked, but isn't it possible to call render again, to create a completely new instance?

you're now forced to split it to multiple test cases

How much to test in a single test is a trade off / heavily opinionated, in my opinion.^^ Without any context I would say that tends to be a good thing.

(<...>Input properties that are not defined are cleared<...>) This is not that big of a deal with smaller components as usually you can pass the same values over and over again but it gets pretty ugly with bigger components that take more complicated objects for their inputs (like form elements, arrays, etc.). Passing these inputs all over again is really not DRY.

That way we can now test the behavior of ngOnChanges. I don't like to use ngOnChanges, but when I do, I often want to test it.

Would it maybe be possible to split your inputs into variables you want to update and those you want to change? Or use some kind of test data builder? That would be DRY and also be more explicit about what will change in your test.

This could be fixed by introducing a new property called partialUpdate

At first glance, I am open to the idea. If agreed within on own issue / feature request I would be willing to create a pull request for it.

[dzonatan]

Thank you for taking time to respond to this.

testing-library is an abstracted approach. Adopting your testing-library/react knowledge to testing-library/angular, testing-libary/... should be obvious. I guess testing-library/react rerender is the guideline with most followers / the reference implementation?

I haven't thought about this much, also I'm not familiar with testing library for react, but yeah.. it makes sense what you're saying. Although, after looking into the React's API and playing around with it I have to say it looks weird 😄 mainly because rerender accepts whole component, the fact that I can rerender completely different component (obviously this happens from scratch) raises more questions for me. So I guess the "ease of adoption" question is not that straightforward as it seems.

Would you mind to create a PR to update it?

Yeah, I can do that, just wanted to raise these thoughts first.

I haven't checked, but isn't it possible to call render again, to create a completely new instance?

Unfortunately, no, render tries to instantiate testing module which can only be done once within Angular, so it fails with: Cannot configure the test module when the test module has already been instantiated.

How much to test in a single test is a trade off / heavily opinionated, in my opinion.^^ Without any context I would say that tends to be a good thing.

I see what you mean. To be honest, after refactoring my test cases that were affected by this chagne I do not have a strong opinion about this anymore.

That way we can now test the behavior of ngOnChanges. I don't like to use ngOnChanges, but when I do, I often want to test it.

Totally agree but previously you could also test ngOnChanges using the change function.

Would it maybe be possible to split your inputs into variables you want to update and those you want to change?

This is exactly what I started doing with this change. But still, with bigger components that have many inputs it makes the test code very verbose and kind of asks for additional comments as the code is no longer self-explanatory.

// you no longer can tell which inputs have actually changed and which you're just passing through
// without tracking every variable to see when/if they were changed before calling `rerender`
await rerender({ componentInputs: { formControl, fooObject, barObject, fooBarObject } });

// previously you could have a good idea of the test case just from this line of code
// because you know that you're changing this one input only:
change({ formControl });

At first glance, I am open to the idea. If agreed within on own issue / feature request I would be willing to create a pull request for it.

If you still feel that way, I can open a new issue (only regarding the properties/input change improvements, I'm not so much interested into the rerender from scratch thing anymore).

[Roman-Simik]

Hi, currently I went also through this "nightmare" of refactoring change() to rerender() and also started wondering if this new behaviour of the rerender() function is right,

• If I understand correctly, with this changes we can properly test the ngOnChanges() hook right?
• But I really didn't have time to manually fix 2000+ tests, so I did little bit of "hack" for now (so we can use the new version) and through iterations write it correctly ->

const spectator = await render({ ..., componentProperties: {...} })

// previous:
spectator.change({ type: 'LISTING' });

// hacky new solution
await spectator.rerender({
  componentProperties: {
    // append all currently existing properties
    ...spectator.fixture.componentInstance,
    type: 'LISTING',
  },
});

• I'm ok with the changes if it was needed, but maybe in the future you will atleast keep the old methods as @deprecated for some time so we have time to refactor it and not just remove them like it's nothing.

[timdeschryver]

Hi, it took me a while to get to this issue, but I see that @shaman-apprentice already has provided more insights on why we made this change.

I'm sorry to hear that this change broke many tests @dzonatan and @Roman-Simik , I think we deprecated it in the last version, but maybe we had to keep it one more version. This is something to keep in mind for the future.

This library aims to be compatible with the other Testing Library projects, and with how Angular interacts with components. But, on the other we also want to bring the joy into writing tests, and making it easier to test Angular applications.

I like the idea of having an overload of rerender (instead of introducing a second method again).
Otherwise other adaptors while face the same issues when they upgraded to the latest version, and I see the "hacky" solution being used more in those projects.

So, the default behavior of rerender should rerender the whole component and its props, but we should make it easier to do a partial update.

await rerender({ componentInputs: { firstName: 'Jacob' }, partialUpdate/partial: true });
await rerender({ componentInputs: { firstName: 'Jacob' }}, true);
await rerender({ componentInputs: { firstName: 'Jacob' }}, { partialUpdate/partial: true }});

What do you think about this?
If we want to go forward with this, we need to first discus the API of it (in a different issue).

To get back on why we can't run render twice, is because (1) it instantiates the testbed, and (2) it doesn't clear the DOM which means that we would end up with the a different instance of the same component in the DOM.

[Henksen]

I have a question about the new rerender method. I ran into an issue in which some child components where not destroyed upon an event. I wanted to simulate the same behavior in testing library. However when I used rerender the components were destroyed, unlike the behavior in the browser. Is it possible that it still rebuilds the renderresult from scratch, instead of just calling the ngChanges? Or maybe the state of injected services are reset? I am sure that something more happens.

I am using version 15.1.0 of testing library/angular.

[shaman-apprentice]

@Henksen can you provide a minimal example for your unwanted behavior? I think I would have time next Weekend to dig into a minimal example

[timdeschryver]

Feel free to create a new issue for this with a reproduction.
It shouldn't destroy the component, but it should invoke ngOnChanges.