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

Not passing an input to a root form can trigger unexpected behavior in some cases #200

Open
maxime1992 opened this issue Dec 30, 2020 · 2 comments

Comments

@maxime1992
Copy link
Contributor

In both

v5:

filter(newValue => !isEqual(newValue, this.formGroup.value)),

And v6:

return options.manualSave$.pipe(
withLatestFrom(formGroup.valueChanges),
map(([_, formValue]) => formValue),
delay(0),
filter(formValue => formGroup.valid && !isEqual(transformedValue, formValue)),
);
} else {
if (options.handleEmissionRate) {
return formGroup.valueChanges.pipe(
options.handleEmissionRate,
delay(0),
filter(formValue => formGroup.valid && !isEqual(transformedValue, formValue)),
);
} else {
return formGroup.valueChanges.pipe(
delay(0),
filter(formValue => formGroup.valid && !isEqual(transformedValue, formValue)),

We do a check on the equality between the previous value of the form and the current one.

If we do not bind the input of a root form (in the case where we care only about the output like a filter) then the old value is never up to date and we can never emit again the default value (which is the case when we clear up the only value changed).

(as an internal reference within our CloudNC repo, @zakhenry check the commit 67be1969d14393ad4d7b4964ca68789f0b539f20 which contains the fix I raised about a month ago for our filter component)

In order to go around this, the input is required and whenever the output of the root form changes, it should come back as the input of it asap. Hence why I'm worried of #196

Not sure what we want to do here though, any idea is welcome.

@zakhenry
Copy link
Contributor

I kinda feel like that is a bug then - the input should be optional, and if omitted maybe it needs to be rebound to the output

@zakhenry
Copy link
Contributor

as in (pseudocode)

(
input$: Observable<T> = output$,
) {}

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

2 participants