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

Partially completed array values with minItems get replaced by defaults #3602

Closed
4 tasks done
RPiAwesomeness opened this issue Apr 13, 2023 · 5 comments
Closed
4 tasks done

Comments

@RPiAwesomeness
Copy link
Contributor

RPiAwesomeness commented Apr 13, 2023

Prerequisites

What theme are you using?

core

Version

5.x

Current Behavior

If an array field has the minItems key set and has at least 1 less item than that, the values are replaced with the defaults. This only occurs when calling the computeDefaults function directly, such as in a test-case.

Expected Behavior

The existing form values are not replaced

Steps To Reproduce

  1. Check out main
  2. Add the following test to packages/utils/test/schema/getDefaultFormStateTest.ts:
it('should combine defaults with raw form data for a required array property with minItems', () => {
    const schema: RJSFSchema = {
      type: 'object',
      properties: {
        requiredArray: {
          type: 'array',
          items: { type: 'string', default: 'default0' },
          minItems: 2,
        },
      },
      required: ['requiredArray'],
    };
    expect(
      computeDefaults(
        testValidator,
        schema,
        {
          rootSchema: schema,
          rawFormData: { requiredArray: ['raw0'] },
          behaviorBitFlags: DefaultFormStateBehavior.IgnoreMinItemsUnlessRequired,
        }
      )
    ).toEqual({ requiredArray: ['raw0', 'default0'] });
  });
});
  1. Confirm the test fails, with the returned value being { requiredArray: ['default0', 'default0'] }

Environment

- OS: Windows 11 (WSL 2)
- Node: v19.4.0
- npm: 9.3.0

Anything else?

This was encountered during work on an MR and was discussed with @heath-freenome. It seems like this doesn't actually affect the functionality of the form in a way I can replicate right now on the Playground, just when testing the computeDefaults function directly. Thus this is more of a placeholder/technical bug than one that is actively impacting the way the library works.

Potentially related to #2980

@RPiAwesomeness RPiAwesomeness added bug needs triage Initial label given, to be assigned correct labels and assigned labels Apr 13, 2023
@RPiAwesomeness
Copy link
Contributor Author

RPiAwesomeness commented Apr 13, 2023

Similar test:

it('should return partial array for an optional array property with minItems when provided with a partial array', () => {
  const schema: RJSFSchema = {
    type: 'object',
    properties: {
      optionalArray: {
        type: 'array',
        minItems: 2,
      },
    },
  };
  expect(
    computeDefaults(testValidator, schema, {
      rootSchema: schema,
      rawFormData: { optionalArray: ['test'] },
      behaviorBitFlags: DefaultFormStateBehavior.IgnoreMinItemsUnlessRequired,
    })
  ).toEqual({ optionalArray: ['test'] });
});

Will fail with the returned value being {optionalArray: [undefined]} because there is not defined default value for the items.

@heath-freenome heath-freenome added help wanted arrays defaults and removed needs triage Initial label given, to be assigned correct labels and assigned labels Apr 14, 2023
@heath-freenome
Copy link
Member

Fixed by #3604

@anthonycaron
Copy link
Contributor

What should be the behavior for an optional array with no default value if we don't have a value for the array ?
rawFormData would have a value of {}.
Not sure if the expected result should be the same: {} or { optionalArray: undefined }. Or something else ?
I would go for { optionalArray: undefined } since it's declared in the schema hence I guess we expect to have in our resulting data but with a value of undefined since it's optional. Also ajv would consider the data as valid in that case.

Here's the test:

it('should return partial array for an optional array property with minItems when provided with a partial array', () => {
  const expectedResult = { optionalArray: undefined };
  const schema: RJSFSchema = {
    type: 'object',
    properties: {
      optionalArray: {
        type: 'array',
        minItems: 2,
        items: { type: 'string' },
      },
    },
  };
  expect(
    computeDefaults(testValidator, schema, {
      rootSchema: schema,
      rawFormData: {},
      behaviorBitFlags: DefaultFormStateBehavior.IgnoreMinItemsUnlessRequired,
    })
  ).toEqual(expectedResult);
});

@heath-freenome
Copy link
Member

@anthonycaron it's debatable honestly. I'm leaning towards expanding the new experimental behavior to give more control over the individual states as best we can.

@anthonycaron
Copy link
Contributor

@heath-freenome Oh I didn't know about this. Sounds good, thanks for your feedback!

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