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

Conflict when using an optional array with minItems set #3363

Closed
3 of 4 tasks
ThomasAribart opened this issue Jan 9, 2023 · 10 comments · Fixed by #3604
Closed
3 of 4 tasks

Conflict when using an optional array with minItems set #3363

ThomasAribart opened this issue Jan 9, 2023 · 10 comments · Fixed by #3604

Comments

@ThomasAribart
Copy link

Prerequisites

What theme are you using?

material-ui

Version

5.x

Current Behavior

Hi and thanks for this great lib 🙌

When setting an optional property in an "object", which is of type "array" with a "minItem" set to 1 or more, it's not possible to remove the property from the said object.

By default, the array property widget appears with items that can be empty, which can cause bugs.

Expected Behavior

I don't have any design proposal, I know it's tricky to represent, but the default items should only appear when deciding that the property is not undefined somehow.

Steps To Reproduce

const mySchema = {
  type: "object,
  properties: {
    optionalArray: {
      type: "array",
      items: { type: "string" },
      minItems: 2
    }
  }
}

Environment

- OS: Any
- Node: Any
- npm: Any

Anything else?

No response

@ThomasAribart ThomasAribart added bug needs triage Initial label given, to be assigned correct labels and assigned labels Jan 9, 2023
@heath-freenome
Copy link
Member

Here's a playground for it

@heath-freenome heath-freenome added good first issue defaults and removed needs triage Initial label given, to be assigned correct labels and assigned labels Jan 13, 2023
@heath-freenome
Copy link
Member

@ThomasAribart Good bug. The code is not taking into account the required-ness of the property. This seems like an easy fix in the @rjsf/utils getDefaultFormState() function's helper computeDefaults(). Are you willing to fix this for us? If so, I can point you in the right direction.

@kuosandys
Copy link
Contributor

Hi there 👋 we’re also encountering the same bug in our project. Is there currently work in progress to fix it? If not, could we offer our help with fixing it?

@heath-freenome heath-freenome added the utils Related to @rjsf/utils label Feb 24, 2023
@RPiAwesomeness
Copy link
Contributor

Same here. Would be willing to help out to get this fixed.

@heath-freenome
Copy link
Member

heath-freenome commented Mar 13, 2023

Yes please! Any help fixing issues is highly encouraged as we are a small team of volunteers who fix things because we want this project to succeed but do so only when we have a little free time.

@RPiAwesomeness
Copy link
Contributor

RPiAwesomeness commented Mar 14, 2023

@heath-freenome Question for you. I'm working on this and ran into a situation where I think one of the existing tests conflicts with this bug. Here's the console output:

image

I think the consensus here is that if the field isn't required and has minItems set, that no items will be automatically added. Correct? In that case, I would argue that if the field isn't required, has minItems set > 0, has no explicit default(s) defined, and no items have been added by the user, then the field shouldn't be included in the output (and thus the default form state) at all. Having an empty array included in the data seems to contradict the minItems requirement. Thus making this test invalid.

Are you okay with me removing this test or is there a part of the JSON Schema spec that I'm missing that specifies this behavior?

@RPiAwesomeness
Copy link
Contributor

RPiAwesomeness commented Mar 14, 2023

Another one that's similar:

image

Seems odd to be auto-filling default values when they're not marked as required.

For clarity's sake, these are more to spark discussion than making a statement one way or the other.

@heath-freenome
Copy link
Member

@RPiAwesomeness These are great questions and merit a verbal conversation with the rest of the current maintainers (all 2 of us). Are you able to come to the weekly meeting that we have every Friday at 11am Eastern time? See this post for the deets

@RPiAwesomeness
Copy link
Contributor

Potentially. I'll double-check my schedule but that should work for me. Anything in particular that I should prep to help the discussion?

@heath-freenome
Copy link
Member

Potentially. I'll double-check my schedule but that should work for me. Anything in particular that I should prep to help the discussion?

Not really, if anything there might be a new minor feature that will need to be added to support the work you are doing in a backwards-compatible way. We can talk about it Friday and give you a bit more background than is here in this bug.

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