This library calls detect changes on every property change

[antischematic]

@Component({
  template: `
    count: {{ count }}
    <button (click)="increment()">Increment</button>
  `
})
export class Counter {
  elementRef = inject(ElementRef)
  count = 0

  increment() {
    this.count++
    this.count++
    this.count++

    expect(elementRef.nativeElement).toHaveTextContent("count: 0")
  }
}

await render(Counter)

fireEvent.click(screen.getByText("Increment"))

Expected result: count: 0
Actual result: count: 3

This is not correct Angular behavior and breaks custom change detection strategies.

Proposal: Use ComponentRef#setInput to change component properties instead.

[timdeschryver]

While I agree that this isn't the Angular behavior, I do expect the test case to behave like this.
Could you share some insights about "breaks custom change detection strategies" for your use case please?

[antischematic]

In my use case I am using proxies to generate a dependency graph and checking if those dependencies are dirty with change detection hooks. Using this with render causes a few problems:

1. I'm unable to write tests that assert change detection runs X number of times, because this lib is calling detectChanges when I'm not expecting it.
2. In some cases this causes infinite recursion in tests that mutate dependencies inside reactive functions.
3. ngOnChanges is called when I'm not expecting it.

https://github.com/antischematic/angular-state-library/blob/35f81fed397af75b87e68681e0be56e026769d79/projects/core/src/tests/invoke.spec.ts

Replace import { render } from "./utils/render" workaround with import {render} from "@testing-library/angular" and run ng test core to reproduce the failing test cases.

[timdeschryver]

I see, thanks for clarifying this @antischematic.
What do you think of adding a config property (that could be set globally), to disable the automatic change detection?
I think for most of the projects, the default should still invoke the change detection.

[antischematic]

I think automatic change detection should be kept while changing the implementation so it aligns better with real app behaviour.

1. Automatic change detection should be done using fixture.autoDetectChanges. This makes it run like a normal app.
2. Passing detectChanges: false should disable automatic change detection.
3. Setting componentProperties or calling change should only set input properties using fixture.componentRef.setInput. This method throws an error when trying to set a non-input property.

See example here.

These changes would require a bump to min Angular version >= 14.1. It should also be possible to get this working with a wrapper component using reflectComponentType but haven't tried it yet.

[timdeschryver]

Thanks again for the input @antischematic .
After toying with it for a while, I do have see some concerns.

setInput fails when the property isn't marked with @Input(), so this would fail for output properties. Sadly, this doesn't throw but it creates a console.error, which makes it harder to fall back to the current behavior.

I've tried autoDetectChanges in the past, but this didn't work the way I wanted it to do. This could have changed now, and is something that I need to investigate soon.

reflectComponentType is interesting, and we could make use of it in some places. Sadly, we also allow the render('<app-component></app-component>') syntax, which is useful for some scenarios but this also means that reflectComponentType does not help a lot here because we don't know the type of the component, and we also create a wrapper component to render it.

I do want to allow a more fine-grained control over the inputs/outputs properties, and the CD.
I'm thinking of adding an componentInputs and componentOutputs render options, which would allow us to implement your suggestions. We could also gradually phase out componentProperties to give a better experience later.

[timdeschryver]

@antischematic feel free to try out the beta version (13.0.0-beta.6).
You can find the changes on the release page.

I will also update our app to this version later today/tomorrow to make sure there are no other breaking changes.

[timdeschryver]

@antischematic sorry to re-ping you but did you had the chance to test the beta release?

[antischematic]

@timdeschryver I've tried it out, here's my feedback:

changeInput doesn't run change detection after it is called.

function changeInput(values)
  for (const [name, value] of Object.entries(values)) {
    fixture.componentRef.setInput(name, value)
  }
  fixture.detectChanges() // call once after all inputs are set, this will also call ngOnChanges.
}

☝️ should be something like this

const {changeInput} = render({ detectChanges: false })

I pass detectChanges: false to render so I can attach listeners before the first render. Then I am manually calling fixture.autoDetectChanges(). I expect changeInput to detect changes in this configuration.

ngOnChanges is called even if I don't pass in componentInputs or componentProperties to render. In a real app ngOnChanges is only called when inputs are actually set.

<counter [count]="0"></counter> <!-- ngOnChanges is called -->
<counter></counter> <!-- ngOnChanges is never called -->

It also appears to be calling ngOnChanges twice when calling changeInput(); fixture.detectChanges().

Other than that it looks good, though it's a bit confusing to have both detectChanges and autoDetectChanges as options to pass to render.

[timdeschryver]

@antischematic good catch about the ngOnChanges firing twice.
I'm not able to reproduce that ngOnChanged fires when there's no inputs though, for example:

• during changeInputs https://github.com/testing-library/angular-testing-library/blob/beta/projects/testing-library/tests/changeInputs.spec.ts#L53
• during change https://github.com/testing-library/angular-testing-library/blob/beta/projects/testing-library/tests/change.spec.ts#L68
• during render https://github.com/testing-library/angular-testing-library/blob/beta/projects/testing-library/tests/render.spec.ts#L255

This version is available as 13.0.0-beta.9 and will probably be released next week

[antischematic]

@timdeschryver Thanks for that, my tests are now passing 13.0.0-beta-9 without any issues.

[timdeschryver]

This is released in v13 🥳
Thanks @antischematic !

[timdeschryver]

@all-contributors please add @antischematic for bug, ideas

[allcontributors]

@timdeschryver

I've put up a pull request to add @antischematic! 🎉