Formsy onChange model missing value object methods

[wkerswell-gresham]

Hi,

For context I have created a datepicker using withFormsy(). The date selected by the user is a Moment.js object which has a bunch of methods for manipulating and formatting the date.

The model on the Formsy prop onChange((model)=>{ }) remove all the __proto__ methods on the object.

I think this is down to the cloneIfObject() method in

formsy-react/src/utils.ts

Line 47 in 6fcc682

export function cloneIfObject(value: unknown) {

which is spreading the current model values.

This doesn't seem to be the same behaviour with setting the value using the setValue() prop in the withFormsy component.

This was working pre the v1 to v2 update.

Is the cloneIfObject method needed?

Many thanks

[rkuykendall]

Thank you for the first post-2.x bug report!

The change appears to date back to formsy 1.0 or just after. However, it's probably been used now in a place or way it wasn't before:

https://github.com/formsy/formsy-react/blame/42e39cbc8fa14333374c56b38b9b9bf389916991/src/index.js#L91-L105

Our options as I see them are to:

1. Avoid cloning, accepting possible param reassignments the original author of the change was trying to keep safe from

2. Improve the function cloneIfObject to not break objects like Moment.js objects

@aesopwolf do you have any opinions on this?

[wkerswell-gresham]

Thanks for the quick reply.

I have done some reading and I think the cloneIfObject spread could be replaced with Object.assign({}, data) which is what was originally there.

[rkuykendall]

It doesn't appear so. I'm going to write a test-case to get more definitive information.

Also, one additional option is that objects like this just shouldn't be supported. In that case, you would want to use something like an ISO 8601 string as your actual "value" and either use moment only internally in your input component, only external to the form, or in both. What do you think about that?

There's some sense in formsy only supporting things that could be sent to or from a server as JSON.

[wkerswell-gresham]

After some playing yesterday I have worked around it by using the model value to create another moment object.

  const onChange = ({date}) =>{
      if(date){
        const momentDate = moment(date);
        ...
      }
  }

Makes sense what you are saying about only supporting the JSON so happy for this issue to be closed.
Thanks

[rkuykendall]

Released:

v2.0.1

• Respect dot notation in reset (Respect dot notation in reset #393) a2d2e5f
• Remove unnecesary externalError state (Remove unnecesary externalError state #398) d4eb73e
• Allow validation errors to be React elements (Allow validation errors to be React elements #394) 4096296
• Update rollup plugin libraries (Update rollup plugin libraries #397) bebefbe
• Improvements to README and PR Template (Improvements to README and PR Template #395) df5af84
• Fix types in Formsy spec tests (Fix types in Formsy spec tests #396) 954370a
• Remove unused Formsy props; tighten types; improve runRules (Remove unused Formsy props; tighten types; improve runRules #392) 0eaeef7
• Rename cloneIfObject to protectAgainstParamReassignment; Fix isObject (Rename cloneIfObject to protectAgainstParamReassignment; Fix isObject #391) 49fc634
• Added possibility to use browser builtin submit (Added possibility to use browser builtin submit #226) 761b3ef
• Improve test coverage (Improve test coverage #378) b8881d0
• Dependabot Automatic Library Bumps (Dependabot Automatic Library Bumps #376) 8b5a0db
• Use React's New Context API (Feature/new context api #198) a79eec2
• Fix in-page link to updateInputsWithValue 6fcc682
• Change coveralls from dependency to dev dependency c9d9db5
• Add coverage to gitignore 0afef60

v2.0.0...v2.0.1

[rkuykendall]

( complex objects should now be safe )