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

fix: mui/material-ui SelectWidget value undefined #3870

Merged

Conversation

Intendanto
Copy link
Contributor

@Intendanto Intendanto commented Sep 16, 2023

Reasons for making this change

fixes #3844

If enumOptions doesnt include value - enumOptionsIndexForValue returns undefined. Mui/MaterialUi uses (useControlled)[https://github.com/mui/material-ui/blob/master/packages/mui-utils/src/useControlled/useControlled.js#L7C52-L7C52] hook to control whether state of component should be internal or external. When undefined passed initially useControlled locks itself with internal state. Thus external updates are ignored. So replacing undefined with emptyValue.

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run npm run test:update to update snapshots, if needed.
    • I've updated docs if needed
    • I've updated the changelog with a description of the PR
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@heath-freenome
Copy link
Member

heath-freenome commented Oct 5, 2023

@Intendanto Thanks for making the fix. Can you update the CHANGELOG.md file to add a description of the fix for both the @rjsf/material-ui and @rjsf/mui themes under the # 5.13.1 section?

@heath-freenome
Copy link
Member

@Intendanto Thanks for making the fix. Can you update the CHANGELOG.md file to add a description of the fix for both the @rjsf/material-ui and @rjsf/mui themes under the # 5.13.1 section?

Nevermind, I'm pushing a fix of my own and will update the CHANGELOG.md accordingly

@heath-freenome heath-freenome merged commit b18d4b5 into rjsf-team:main Oct 6, 2023
4 checks passed
heath-freenome added a commit to heath-freenome/react-jsonschema-form that referenced this pull request Oct 6, 2023
…d validator-ajv8

Fixes rjsf-team#3671 by resolving refs inside of `properties` and array `items` and restoring `100%` unit test coverage for `utils` and `validator-ajv8`
- Updated `package.json` to add `@types/jest` to the global `devDependencies`
- In `@rjsf/utils`, fixed rjsf-team#3671 and restored 100% test coverage as follows:
  - Updated `jest.config.js` to restore test coverage threshold to 100%
  - Updated `resolveAllReferences()` to take a new `recurseList: string[]` parameter that is used to prevent recurse `$ref` processing
    - Return the `schema` directly when it is not an object and also return the original `schema` when the `resolvedSchema` is identical
  - Updated the `resolveReference()` function to call `resolveAllReferences()` and only call `retrieveSchemaInternal()` when the the `updatedSchema` is different than the original
  - Updated the `resolveSchema()` function to always call `resolveReference()` and only return the `updatedSchemas` when something was changed
  - Updated `retrieveSchemaInternal()` to take a new `recurseList: string[] = []` parameter that is passed to `resolveSchema()` and `resolveCondition()`
    - Updated many of the internal functions to take a `recurseList: string[]` parameter that is forwarded to any function that ultimately calls `retrieveSchemaInternal()` or `resolveAllReferences()`
  - Updated the `SchemaParser` to properly pass the `recurseList` array to `retrieveSchemaInternal()` and `resolveAnyOrOneOfSchemas()`
  - Updated the `getClosestMatchingOption()` function to pass an empty array to the `resolveAllReferences()` function for `recurseList`
  - Updated the `computeDefaults()` function to pass an empty array to the `resolveDependencies()` function for `recurseList`
    - Also removed an unnecessary `isObject()` check for `formData` in when dealing with `additionalProperties` since it is always guaranteed to be an object
  - Added/updated tests to ensure that all of the `@rjsf/utils` have 100% test coverage
- In `@rjsf/validator-ajv8` to fix tests due to the rjsf-team#3671 changes and restored 100% test coverage as follows:
  - Updated `jest.config.js` to restore test coverage threshold to 100%
  - Updated the tests for `precompiledValidator` to call `retrieveSchema()` on the precompiled `rootSchema` to avoid errors caused by the `retrieveSchema()` changes
  - Updated `validator.ts` to make the `isValid()` call not check if the `rootSchema` already exists (because it doesn't) since the `finally` always removes it
    - Updated the `validator` tests to restore 100% test coverage
- Updated the `CHANGELOG.md` accordingly while also adding the description for rjsf-team#3870
@Intendanto
Copy link
Contributor Author

@heath-freenome
Sorry, missed your messages
Ty for merging!

heath-freenome added a commit to heath-freenome/react-jsonschema-form that referenced this pull request Oct 6, 2023
…d validator-ajv8

Fixes rjsf-team#3671 by resolving refs inside of `properties` and array `items` and restoring `100%` unit test coverage for `utils` and `validator-ajv8`
- Updated `package.json` to add `@types/jest` to the global `devDependencies`
- In `@rjsf/utils`, fixed rjsf-team#3671 and restored 100% test coverage as follows:
  - Updated `jest.config.js` to restore test coverage threshold to 100%
  - Updated `resolveAllReferences()` to take a new `recurseList: string[]` parameter that is used to prevent recurse `$ref` processing
    - Return the `schema` directly when it is not an object and also return the original `schema` when the `resolvedSchema` is identical
  - Updated the `resolveReference()` function to call `resolveAllReferences()` and only call `retrieveSchemaInternal()` when the the `updatedSchema` is different than the original
  - Updated the `resolveSchema()` function to always call `resolveReference()` and only return the `updatedSchemas` when something was changed
  - Updated `retrieveSchemaInternal()` to take a new `recurseList: string[] = []` parameter that is passed to `resolveSchema()` and `resolveCondition()`
    - Updated many of the internal functions to take a `recurseList: string[]` parameter that is forwarded to any function that ultimately calls `retrieveSchemaInternal()` or `resolveAllReferences()`
  - Updated the `SchemaParser` to properly pass the `recurseList` array to `retrieveSchemaInternal()` and `resolveAnyOrOneOfSchemas()`
  - Updated the `getClosestMatchingOption()` function to pass an empty array to the `resolveAllReferences()` function for `recurseList`
  - Updated the `computeDefaults()` function to pass an empty array to the `resolveDependencies()` function for `recurseList`
    - Also removed an unnecessary `isObject()` check for `formData` in when dealing with `additionalProperties` since it is always guaranteed to be an object
  - Added/updated tests to ensure that all of the `@rjsf/utils` have 100% test coverage
- In `@rjsf/validator-ajv8` to fix tests due to the rjsf-team#3671 changes and restored 100% test coverage as follows:
  - Updated `jest.config.js` to restore test coverage threshold to 100%
  - Updated the tests for `precompiledValidator` to call `retrieveSchema()` on the precompiled `rootSchema` to avoid errors caused by the `retrieveSchema()` changes
  - Updated `validator.ts` to make the `isValid()` call not check if the `rootSchema` already exists (because it doesn't) since the `finally` always removes it
    - Updated the `validator` tests to restore 100% test coverage
- Updated the `CHANGELOG.md` accordingly while also adding the description for rjsf-team#3870
heath-freenome added a commit that referenced this pull request Oct 10, 2023
…or-ajv8 (#3895)

Fixes #3671 by resolving refs inside of `properties` and array `items` and restoring `100%` unit test coverage for `utils` and `validator-ajv8`
- Updated `package.json` to add `@types/jest` to the global `devDependencies`
- In `@rjsf/utils`, fixed #3671 and restored 100% test coverage as follows:
  - Updated `jest.config.js` to restore test coverage threshold to 100%
  - Updated `resolveAllReferences()` to take a new `recurseList: string[]` parameter that is used to prevent recurse `$ref` processing
    - Return the `schema` directly when it is not an object and also return the original `schema` when the `resolvedSchema` is identical
  - Updated the `resolveReference()` function to call `resolveAllReferences()` and only call `retrieveSchemaInternal()` when the the `updatedSchema` is different than the original
  - Updated the `resolveSchema()` function to always call `resolveReference()` and only return the `updatedSchemas` when something was changed
  - Updated `retrieveSchemaInternal()` to take a new `recurseList: string[] = []` parameter that is passed to `resolveSchema()` and `resolveCondition()`
    - Updated many of the internal functions to take a `recurseList: string[]` parameter that is forwarded to any function that ultimately calls `retrieveSchemaInternal()` or `resolveAllReferences()`
  - Updated the `SchemaParser` to properly pass the `recurseList` array to `retrieveSchemaInternal()` and `resolveAnyOrOneOfSchemas()`
  - Updated the `getClosestMatchingOption()` function to pass an empty array to the `resolveAllReferences()` function for `recurseList`
  - Updated the `computeDefaults()` function to pass an empty array to the `resolveDependencies()` function for `recurseList`
    - Also removed an unnecessary `isObject()` check for `formData` in when dealing with `additionalProperties` since it is always guaranteed to be an object
  - Added/updated tests to ensure that all of the `@rjsf/utils` have 100% test coverage
- In `@rjsf/validator-ajv8` to fix tests due to the #3671 changes and restored 100% test coverage as follows:
  - Updated `jest.config.js` to restore test coverage threshold to 100%
  - Updated the tests for `precompiledValidator` to call `retrieveSchema()` on the precompiled `rootSchema` to avoid errors caused by the `retrieveSchema()` changes
  - Updated `validator.ts` to make the `isValid()` call not check if the `rootSchema` already exists (because it doesn't) since the `finally` always removes it
    - Updated the `validator` tests to restore 100% test coverage
- Updated the `CHANGELOG.md` accordingly while also adding the description for #3870
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

Successfully merging this pull request may close these issues.

Mui SelectWidget: controlled/uncontrolled
2 participants