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

detectChanges is being triggered from setInput and setRouteParam #574

Open
Adam-Pond opened this issue Jul 28, 2022 · 6 comments
Open

detectChanges is being triggered from setInput and setRouteParam #574

Adam-Pond opened this issue Jul 28, 2022 · 6 comments

Comments

@Adam-Pond
Copy link

Is this a regression?

Yes

Description

After setting up a component with detectChanges disabled (set to false in createRoutingFactory or createComponentFactory), detectChanges is called which triggers the ngOnInit to be called. If however, other setup needs to be set for the test to work during initialisation, there is now no opportunity to set the second. The second call does not trigger ngOnInit as it's already been called.

There is a workaround for setInput by setting multiple input parameters concurrently in the one statement

    spectator.setInput({
      param1: param1data,
      param2: param1data,
    });

But, this syntax is not available on setRouteParam.

Regardless, this is a regression and the fix is not to add additional multi-parameter overload to setRouteParam, but rather to respect the fact that it's not the framework's responsibility to call detectChnages when it's been disabled.

Please provide a link to a minimal reproduction of the bug

No response

Please provide the exception or error you saw

Stepping through in the debugger, it's easy to observe this behaviour.

Please provide the environment you discovered this bug in

Angular 14.1.0
"@ngneat/spectator": "^11.1.3",

Anything else?

fit('should not call ngOnInit before both route params are called', () => {
  // arrange
  spectator.setRouteParam('routeParam1', "routeParamValue2");
  // -- [ do some other setup that perhaps injects a service ] --
  
  // act
  spectator.detectChanges();  // expect that in ngOnInit, that further configuration can be done after setRouteParam but before detectChanges

  // never triggers ngOnInit because the first setRouteParam triggered
});

Do you want to create a pull request?

No

@Adam-Pond
Copy link
Author

This seems to be the culprit at a glance

    /**
     * Updates the route params and triggers a route navigation.
     */
    setRouteParam(name, value) {
        if (this.checkStubPresent()) {
            this.activatedRouteStub.setParam(name, value);
            this.triggerNavigationAndUpdate();   // <-- Not sure why setRoutParam should trigger a route navigation. Has this been added since previous versions?
        }
    }
    
    triggerNavigationAndUpdate() {
        this.activatedRouteStub.triggerNavigation();
        this.detectChanges();
    }

@localpcguy
Copy link

Can confirm I ran into this with regards to setInput and detectChanges: false in a really simple test. In my case I was able to work around it with the ability mentioned in the OP to set multiple inputs at once, but ideally these methods should not call detectChanges if it is set to false.

@johant87
Copy link

johant87 commented Dec 23, 2022

I found the same issues after updating from spectator 10.0.0, I feel this change is the differentiator:
#539
c40a2a6
But perhaps this is working as intended as normally onInput change would also trigger change detection.

@NetanelBasal
Copy link
Member

You're welcome to submit a PR

@gelsogrove
Copy link

any news ??? please let us know

@MCD1987
Copy link

MCD1987 commented Jun 17, 2023

You could use SpectatorRouting.triggerNavigation()

This method allows you to set multiple params, queryParams, data etc. via the RouteOptions.
It does not solve the problem if you need to set both input + params since it does triggers a route navigation..

For example:

    SpectatorRouting.triggerNavigation(
      {
        params: {
          testParam1: 'value1', 
          testParam2: 'value2'
        }
      }
    );

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

No branches or pull requests

6 participants