Skip to content

Conversation

mohith2883
Copy link

Fixes #1513

Description

Enhances onChange handling to correctly support clearedValue: "" as a valid value instead of treating it as undefined. Ensures cleared fields with empty strings remain in getState().values. Corresponding tests updated.

Checklist: (please see documentation page for more information)

  • Yarn build passes
  • Yarn lint passes
  • Yarn test passes
  • Test coverage for new code (if applicable)
  • Documentation update (if applicable)
  • Correct commit message
    • format fix|feat({scope}): {description}
    • i.e. fix(pf3): wizard correctly handles next button
    • fix will release a new _._.X version
    • feat will release a new _.X._ version (use when you introduce new features)
      • we want to avoid any breaking changes, please contact us, if there is no way how to avoid them
    • scope: package
    • if you update the documentation or tests, do not use this format
      • i.e. Fix button on documenation example page

@vercel
Copy link

vercel bot commented Oct 11, 2025

@mohith2883 is attempting to deploy a commit to the data-driven-forms Team on Vercel.

A member of the Team first needs to authorize it.

@mohith2883
Copy link
Author

Hey @rvsia , @ssav7912 i have raised a PR
Can you check it

@Hyperkid123
Copy link
Member

@mohith2883, there are a few tests that are failing. Can you address these?

Summary of all failing tests
FAIL packages/react-form-renderer/src/tests/form-renderer/render-form.test.js (6.169 s)
  ● renderForm function › #clearOnUnmount › should clear values after unrender and set to field cleared value

    expect(jest.fn()).toHaveBeenCalledWith(...expected)

    - Expected
    + Received

      Object {
    -   "foo": "barrr",
    +   "foo": "BlaBlaBlabarrr",
        "unmnounted": "bla",
      },

    Number of calls: 1

      1185 |       await userEvent.click(screen.getByText('Submit'));
      1186 |
    > 1187 |       expect(onSubmit).toHaveBeenCalledWith({ foo: 'barrr', unmnounted: 'bla' });
           |                        ^
      1188 |     });
      1189 |
      1190 |     it('should clear values after unrender and set to form cleared value', async () => {

      at _callee26$ (packages/react-form-renderer/src/tests/form-renderer/render-form.test.js:1187:24)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:294:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:119:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)

  ● renderForm function › #clearOnUnmount › should clear values after unrender and set to form cleared value

    expect(jest.fn()).toHaveBeenCalledWith(...expected)

    - Expected
    + Received

      Object {
    -   "foo": "barrr",
    +   "foo": "BlaBlaBlabarrr",
        "unmnounted": "BlaBlaBla",
      },

    Number of calls: 1

      1235 |       await userEvent.click(screen.getByText('Submit'));
      1236 |
    > 1237 |       expect(onSubmit).toHaveBeenCalledWith({ foo: 'barrr', unmnounted: 'BlaBlaBla' });
           |                        ^
      1238 |     });
      1239 |   });
      1240 |

      at _callee27$ (packages/react-form-renderer/src/tests/form-renderer/render-form.test.js:1237:24)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:294:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:119:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)

FAIL packages/common/src/tests/select/select.test.js
  ● Select test › multi select › selects null value - clears selection

    expect(received).toEqual(expected) // deep equality

    Expected: []
    Received: ""

      319 |
      320 |       expect(state.value).toEqual([]);
    > 321 |       expect(inputValue).toEqual([]);
          |                          ^
      322 |     });
      323 |
      324 |     it('selects all values', async () => {

      at _callee7$ (packages/common/src/tests/select/select.test.js:321:26)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:294:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:119:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)

  ● Select test › multi select › with select all and select none at that same time

    expect(received).toEqual(expected) // deep equality

    Expected: []
    Received: ""

      426 |
      427 |       expect(state.value).toEqual([{ label: 'Select none', selectNone: true, value: 'select-none' }]);
    > 428 |       expect(inputValue).toEqual([]);
          |                          ^
      429 |     });
      430 |   });
      431 |

      at _callee10$ (packages/common/src/tests/select/select.test.js:428:26)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:294:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:119:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)

FAIL packages/react-form-renderer/src/tests/form-renderer/enhanced-on-change.test.js
  ● #enhancedOnChange › #setting cleared value › should not set any delete value after sending empty value

    expect(received).toEqual(expected) // deep equality

    Expected: undefined
    Received: "this is deleted value"

      57 |     it('should not set any delete value after sending empty value', () => {
      58 |       const value = undefined;
    > 59 |       expect(enhancedOnChange({ onChange: (value) => value, clearedValue }, value)).toEqual(undefined);
         |                                                                                     ^
      60 |     });
      61 |
      62 |     it('should set delete value after sending empty string value', () => {

      at Object.<anonymous> (packages/react-form-renderer/src/tests/form-renderer/enhanced-on-change.test.js:59:85)


Test Suites: 3 failed, 91 passed, 94 total
Tests:       5 failed, 2 skipped, 1323 passed, 1330 total
Snapshots:   33 passed, 33 total
Time:        91.965 s

Copy link
Member

@Hyperkid123 Hyperkid123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the failing tests, I think we need a little bit more investigation to ensure we are not introducing regressions.

}

if (checkEmpty(result) && typeof initial !== 'undefined') {
if (checkEmpty(result)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this is going to solve the issue without breaking existing behavior. I think we need to look at where the clearedValue is set/retrieved from, and if it really has the "" value when the onChange is called.

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.

clearedValue with empty string is equivalent to undefined.

2 participants