0

It is an interesting behaviour I tried in multiple projects and gave up using OnPush for FormArray, more specific it is an arry of row of FormGrop contains FormControl. However, I think it is time to raise a question here so that it maybe same issue for others (or just me too stupid that forget a simple tiny line of code, please point it out if so, thanks), I created a mockup here: stackblitz

For some reason my project using OnPush for all such dummy componnents which needs manually call ChangeDetectionRef.markForCheck if a View Update is required. But this time wherever I put the mark it doesn't show any differece.

Generally, I created a save$ Subject to do a validation on the fly and save the whole Grid to somewhere if it is valid. Now, since save$ is a BehaviourSubject which means it is called once the form is built. You can see the whole FormGroup is invalid on the screen but all cells are valid, which is weird. enter image description here

Vincent
  • 1,178
  • 1
  • 12
  • 25
  • I think it's from your code. Changing it back to Default will trigger ng100 error. And if you leave OnPush and trigger CD yourself (via a button), you will see it reset the whole state, error -> null -> form become valid whenever CD run. – Jimmy Nov 25 '22 at 21:50
  • @Jimmy Any idea how it should be refactored? – Vincent Dec 08 '22 at 02:18
  • I could give you the simplest, also a hack for this: setTimeout. Use setTimeout when you call control.setError. `setTimeout(() => { control.setErrors(Object.keys(errors).length ? errors : null); })` – Jimmy Dec 08 '22 at 18:13
  • If you want more details, just let me know, I will write the answer – Jimmy Dec 08 '22 at 18:14
  • @Jimmy many thanks for your comments, but the first time build grid still shows valid on each row and the grid becomes valid, which is not right. – Vincent Dec 09 '22 at 23:00
  • hmm, did you call the change detection? or switch to default mode? Without OnPush, it will work as expected. When you use OnPush, you need to trigger CD yourself, so try to create a button that invoke detectChanges function to see if it work. – Jimmy Dec 11 '22 at 11:43
  • @Jimmy Thanks, the question is for OnPush.. not for non-OnPush. And I am trying to find how trigger CD inside the code not manually. So posted this question. The thing is no matter where I put CD, it won't work – Vincent Dec 12 '22 at 00:16
  • I looked up your stackblitz example. The only thing I see different than the Default change detection strategy, is that the form group does not update its "valid" value right after generating the form dynamically. Since the generation is happening in the typescript file, this is a normal behavior. If that is the only problem as I understand, then I could write an answer for it. Please confirm – Mehyar Sawas Dec 12 '22 at 09:06
  • @MehyarSawas please add one answer if you have a cleaner way, thanks! – Vincent Dec 12 '22 at 23:38

1 Answers1

3

The reason I mentioned about "non onpush" is because with that mode, Angular runs CD for you. And if your code works as expected in default mode, it means, your "onpush" code just needed to trigger CD.
I didn't mean to tell you that you should switch to default mode.

For why it didn't work in your original question:
The form itself, by default run updateValueAndValidity after initialized, you don't declare any validators => the form status is valid; then you call setErrors which behind the scene, set the from status to false => this cause the inconsistency in the view, hence the ng100 error.

//Edit:
I debugged the flow again and found out that after each control gets added to the form, it will run the updateValueAndValidity function again. Which means, the setErrors will cause the form to be Invalid first, then updateValueAndValidity will make it all Valid again. The inconsistency is still there, just the other way around.
//End Edit

The dirty hack for this is using setTimeout, which I personally don't like and don't recommend. But to give a properly solution, it will require to know the business logic in the code.

Here it is the code I posted in the comment.

setTimeout(() => {
        control.setErrors(Object.keys(errors).length ? errors : null);        
});

This helps to schedule the setError (which actually, the valid status) to run in the next CD round.
This code doesn't have any CD yet. So yes, the form is valid all the time => That's the reason I tell you to create a button to see the CD in action.

Now we wouldn't want a new button for users to click to see the form update. We need to run the CD in code. So where to put it?
We know the setError trigger the form status so we will schedule it later, and where does the setError get invoked? customValidator, we will schedule this function to run later so the final code to modify is here:

//instead of run setTimeout in a loop
//we make sure that all setError is scheduled in one run
setTimeout(() => {
      customValidator()(form);
      this.cd.markForCheck();
    })
Jimmy
  • 1,276
  • 7
  • 8
  • Thanks, here is why I desire a cleaner code: the business logic behind the scene is 1. Using OnPush on every sub-components like this in the post; 2. Provide that the Grid is created and validator runs, it should immediately show error on the UI if any data corrupted. 3. any input changes also triggers the same validation. 4. Validator is static function as runs as needed (programmatically to improve performance) – Vincent Dec 13 '22 at 01:02
  • edited: I basically have same problem with: https://stackoverflow.com/questions/49125286/why-markforcheck-and-settimeout; so ideally a refactor/less convoluted way is better.. as you mentioned, after initialized -> how I declare the form validity also keep UI updated.. also -> after setErrors, how to update validity again with keep UI in sync... – Vincent Dec 13 '22 at 01:12
  • hmm, not sure why you mentioned those things. Did the new code work for you? – Jimmy Dec 13 '22 at 09:50
  • Just replying this: “But to give a properly solution, it will require to know the business logic in the code”. Still thinking setTimeout is not a proper solution and just like you said should not be best practice. – Vincent Dec 14 '22 at 02:35
  • I prefer built in functions so I just copy your `customValidator` to `formValidator` and put it under `formGroup (dataGrid)` itself. You could check this forked link here: https://stackblitz.com/edit/angular-ivy-hdbrbv?file=src/app/app.component.ts – Jimmy Dec 14 '22 at 18:50
  • The idea behind this is we want the form to work as the way it is currently designed for. When the form done initialized, it invokes `updateValueAndValidity`, which in turns set the whole validate status and errors based on the validators. So adding a validator to the form itself is more properly than setTimeout – Jimmy Dec 14 '22 at 18:54
  • Thanks, that is better over setTimeout + run validator later, which seems more redundant code and not design for. I will take this answer. Thanks! – Vincent Dec 14 '22 at 22:42