uniforms icon indicating copy to clipboard operation
uniforms copied to clipboard

Optional defaultValue does not populate form model, but field only

Open Floriferous opened this issue 2 years ago • 10 comments

As you can see in this reproduction: https://codesandbox.io/s/jolly-perlman-ntuxi

Both fields in the form are filled with their defaultValue, yet only string1 is set in the form model. This causes weird bugs as we rely on the model being exactly what is shown on screen.

This is the schema in question:

const schema = new SimpleSchema({
  string1: {
    type: String,
    defaultValue: "yooo" // Shown in field and form model
  },
  string2: {
    type: String,
    defaultValue: "yooo", // Shown in field only
    optional: true
  }
});

Floriferous avatar Nov 05 '21 17:11 Floriferous

Hi @Floriferous , thanks for the issue report and the reproduction 🙏

It does indeed look like the behavior is inconsistent and should be adjusted. It seems like this part of code is preventing the model from being set properly. We'll have to investigate and see how we can adjust this.

wadamek65 avatar Nov 09 '21 12:11 wadamek65

This is definitely not a bug, and I'm on a fence whether it's an inconsistency. Since the first version of uniforms, we assumed that optional fields don't have to be populated in the model, as the schema will fill them in during cleaning/validation. This topic was tackled multiple times, most recently in https://github.com/vazco/uniforms/issues/875#issuecomment-792707921.

In general, I do agree that in some cases it's useful to have the "full" (i.e., cleaned) model in the state. On the other, none of the schemas I know have a way to "de-default" an object, i.e., remove the default value that would be added anyway.

@Floriferous, @wadamek65 - what's your take on this one?

radekmie avatar Nov 09 '21 12:11 radekmie

Well to me there are really only 2 options:

  • You set the default value in the field and in the model
  • You don't set the default value, neither in the field, nor in the model

Right now it's a mix of both.

The problem for us is the following: Some of our fields depend on a field having a certain value (in the model). There is no other way for us to know the value of that field other than through the model.

So here's how the bug goes:

  • You land on this form with a defaultValue in field1 (which we don't want to remove from the schema, for business reasons)
  • Another field, field2, appears only when field1 has its value set to defaultValue. field2 is required
  • You try to submit the form, and it fails because field2 is required, but the field is not shown, so you can't see what the error is either

At this point, the only way to make field2 appear, is to change the value of field1 twice (!!), because you first need to set it on the model by choosing another value, and then you set it back to defaultValue for field2 to appear... clearly an impossible situation.

In any case, your view doesn't match your model at this point..

Floriferous avatar Nov 09 '21 16:11 Floriferous

Sorry for not answering for so long. We've had an internal discussion about that, and it's (most probably) going to be changed in v4. We do have a few more things to go through to make sure nothing will break. We'll keep this issue updated.

radekmie avatar Nov 24 '21 15:11 radekmie

(We've had a team discussion about this issue, and here are the notes.)

We've decided that in v4 all defaultValues will be initialized in the model immediately.

radekmie avatar Dec 10 '21 10:12 radekmie

Bumping, this change is really important for us and how we use these forms :)

Floriferous avatar Jul 28 '22 14:07 Floriferous

@Floriferous We've planned a meeting about uniforms v4 in the following weeks, but no ETA for any of the features yet, sorry.

radekmie avatar Aug 09 '22 07:08 radekmie

And here we are with fresh updates on defaultValue in uniforms@4;

  • We will always set the default value for all - required and non-required fields (if provided).
  • We will add a getInitialModel to detach the model state from the render function.

@Floriferous We still don't have ETA, but I think this ticket should be somewhere on the top of the list. We will keep posting updates here.

Monteth avatar Sep 02 '22 12:09 Monteth

Hi guys, this inconsistency doesn't make sense and leads to bugs. I didn't know about this behavior, so I shipped app where a value was visibly set by defaultValue but not in model. This disabled a functionality. It really shouldn't be in this state where you can see something different in UI than in underlaying state. How is it even possible in react-like nature and environtment? Could this, please, be fixed sooner than v4?

mariusrak avatar Mar 17 '23 16:03 mariusrak

Hi guys, this inconsistency doesn't make sense and leads to bugs.

We know, that's why we decided to change it in v4.

How is it even possible in react-like nature and environtment?

Honestly, it's because we've never encountered it ourselves.

Could this, please, be fixed sooner than v4?

Sorry, it cannot, because it's a backward-incompatible change.

radekmie avatar Mar 26 '23 20:03 radekmie