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

Unwanted default value for object field (regression introduced in 5.22.0) #4361

Open
4 tasks done
AlimovSV opened this issue Nov 1, 2024 · 7 comments
Open
4 tasks done

Comments

@AlimovSV
Copy link
Contributor

AlimovSV commented Nov 1, 2024

Prerequisites

What theme are you using?

core

Version

5.22.3

Current Behavior

If a schema contains an object field with subschema oneOf: { const: 123 } and emptyObjectFields <> 'skipDefaults', then the default value is applied as the first value from the oneOf subschema.

Expected Behavior

Object fields should not be populated if default value is not specified in the schema

Steps To Reproduce

Open playground: you will see a simple schema with one property WITHOUT default in the schema, but the RJSF set the default value as a first value from oneOf subschema. You can see this by trying to edit the form data (just try renaming prop -> prop2 and prop: 123 will be added automatically).

Environment

- OS:
- Node:
- npm:

Anything else?

No response

@AlimovSV AlimovSV added bug needs triage Initial label given, to be assigned correct labels and assigned labels Nov 1, 2024
@heath-freenome
Copy link
Member

heath-freenome commented Nov 1, 2024

@AlimovSV I'm not totally convinced it is a regression, but rather the result of fixing const so that it behaves like default in terms of populating default data (a very OLD bug). If you were to switch your const to be an enum with a default then you would get the same behavior.

You can also not make the const be required. If you need it to be a required field then you can simply set the experimental_defaultFormStateBehavior.emptyObjectFields' on Formto beskipDefaults`.

If that isn't acceptable, then let's discuss more.

This is probably not the first time that people rely on a bug as if it was a feature. And then when the bug get fixed suddenly there is a "regression".

@heath-freenome heath-freenome added awaiting response defaults and removed needs triage Initial label given, to be assigned correct labels and assigned labels Nov 1, 2024
@AlimovSV
Copy link
Contributor Author

AlimovSV commented Nov 2, 2024

@heath-freenome Thank you for response. I think this is still a bug because:

  1. form doesn't have a default value
  2. the schema transforms into the select box (so the user has several options to choose from) but with a predefined first option

So the case where I want to force the user to make an initial choice is not possible. RJSF makes its choice. skipDefaults is not a good variant because it removes all defaults even for fields with default: x in the schema. I think that skipEmptyDefaults would be helpful, but it doesn't work for oneOf case too.

If you were to switch your const to be an enum with a default.

Yes, and this behavior is correct, because I have "default: 1" in the schema. But in my example the schema does not have a default. I agree that if the schema is const: 123 only (without oneOf choice), it is correct, because the user has no possibility to select something else, but when the schema contains a choice (oneOf or enum) and does not have a default - the automatic selection of the first possible value is not so good (why not the last or other one?). If I want the default for this case I must specify it in the schema.

@heath-freenome
Copy link
Member

Hmmm, so maybe we add a new experimental_defaultFormStateBehavior flag to disable automatic const assignment as a default?

@AlimovSV
Copy link
Contributor Author

AlimovSV commented Nov 6, 2024

as a variant - may be. but I still think that the schema

{
  "type": "object",
  "required": [
    "prop"
  ],
  "properties": {
    "prop": {
      "const": 1
    }
  }
}

should have automatic const assignment, but this schema should not have it:

{
  "type": "object",
  "required": [
    "prop"
  ],
  "properties": {
    "prop": {
      "type": "string",
      "oneOf": [
        {
          "const": "1",
          "title": "One"
        },
        {
          "const": "2",
          "title": "Two"
        },
        {
          "const": "3",
          "title": "Three"
        }
      ]
    }
  }
}

just because we are not 100% sure what value should be assigned to the data ("1", "2" or "3").

@nickgros
Copy link
Contributor

nickgros commented Nov 8, 2024

@AlimovSV what do you think of the proposal in this comment? Would the skipOneOf flag solve this issue for you?

@AlimovSV
Copy link
Contributor Author

AlimovSV commented Nov 9, 2024

@nickgros Yes, thank you, I like it!

heath-freenome added a commit to heath-freenome/react-jsonschema-form that referenced this issue Nov 14, 2024
…ression

Fixes rjsf-team#4344, rjsf-team#4361 and rjsf-team#4377
- In `@rjsf/utils`:
  - Updated the `Experimental_DefaultFormStateBehavior` type to add new optional `constAsDefaults` prop with three choices
  - Updated `getDefaultFormState()` to respond to the new `constAsDefaults` feature to limit `const` as defaults based on the `never` or `skipOneOf` choices
  - Added tests for `getDefaultFormState()` to verify the new feature
- In `@rjsf/core`:
  - Updated `SchemaField` to remove making the field readonly when const
- In `playground`:
  - Updated `Header` to add support for `constAsDefaults`
- Updated the `CHANGELOG.md` accordingly
heath-freenome added a commit that referenced this issue Nov 15, 2024
…ression (#4381)

Fixes #4344, #4361 and #4377
- In `@rjsf/utils`:
  - Updated the `Experimental_DefaultFormStateBehavior` type to add new optional `constAsDefaults` prop with three choices
  - Updated `getDefaultFormState()` to respond to the new `constAsDefaults` feature to limit `const` as defaults based on the `never` or `skipOneOf` choices
  - Added tests for `getDefaultFormState()` to verify the new feature
- In `@rjsf/core`:
  - Updated `SchemaField` to remove making the field readonly when const
- In `playground`:
  - Updated `Header` to add support for `constAsDefaults`
- Updated the `CHANGELOG.md` accordingly
@AlimovSV
Copy link
Contributor Author

@nickgros Yet another issue I observed: when const: has $data specs (using AJV $data: true configuration - unfortunately the playground doesn't allow to configure AJV and validation will not work properly but this is not important), the constAsDefault logic populates it in naive manner - just copy the value. And the email confirmation becomes [object Object] in the UI and { "$data": "1/email" } in the form data. Of course when I make constAsDefault="never" this behavior is gone, but I think it would be worth to handle such a case in some alternative manner. Thoughts?

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

No branches or pull requests

3 participants