jaredpalmer / formik

Build forms in React, without the tears 😭
https://formik.org
Apache License 2.0
33.98k stars 2.79k forks source link

Formik not updating values when order of keys in object changes #3329

Open tameem-hamoui opened 3 years ago

tameem-hamoui commented 3 years ago

Bug report

Current Behavior

I'm trying to re-order an object in order to sort some fields. Using formik it sorts the first time but then it seems to do some kind of caching and wont refresh the order anymore. I cannot change the structure to an array.. It has to be an object. The only other thing I thought of doing was adding some random value to the object but that wont pass code review. Is there anything else I can do to get this object to sort when passed through formik?

// variationsByKey is a new object each time but has the same keys values as the old object (but in a diff order)
  console.log('before', variationsByKey);
  return (
    <Formik
      enableReinitialize={true}
      initialValues={variationsByKey}
      {({
        values,
      }) => {
        console.log('after', values);

Sorts a few times and then keeps memory of the object. before is the correct sorting, and after ends up never changing.

before {new_variation: {…}, on: {…}, off: {…}}

after {new_variation: {…}, off: {…}, on: {…}}

Expected behavior

When a new object is passed into initialValues formik should re-render even if the previous object has keys and values matching the old object.

Reproducible example

https://codesandbox.io/s/sweet-brattain-29z53?file=/src/App.js

Suggested solution(s)

If the order of the keys changes in an object then the values should be refreshed.

Additional context

I'm trying to solve this difficult problem at work where sorting is broken but I have no control over the input to formik. We are changing the order of the keys in these objects and it sorts in every other table not wrapped in formik.

Your environment

Software Version(s)
Formik ^2.1.4
React 16.14.0
TypeScript
Browser chrome
npm/Yarn yarn
Operating System mac os big sur
johnrom commented 3 years ago

If you use an array instead of an object it should work. the key of the values object exists already as the key property of the value itself, so you should be able to call object.values on it and still do what you need to do.

I'm assume Formik's isEqual check doesn't compare the "sorting" of objects.

johnrom commented 3 years ago

You can see the workaround in action here: https://codesandbox.io/s/angry-payne-q3njf?file=/src/App.js

For what it's worth I know that JS allows developers to use an object like an sorted, iterable array, but I prefer to treat it more like a key-indexed, unsorted bag of properties, especially because destructuring and spreading mean that objects are often in different orders even when they should be considered "equivalent" from a performance standpoint.

tameem-hamoui commented 3 years ago

You can see the workaround in action here: https://codesandbox.io/s/angry-payne-q3njf?file=/src/App.js

For what it's worth I know that JS allows developers to use an object like an sorted, iterable array, but I prefer to treat it more like a key-indexed, unsorted bag of properties, especially because destructuring and spreading mean that objects are often in different orders even when they should be considered "equivalent" from a performance standpoint.

Yeah this workaround works but now youre using an array vs an object. That totally makes sense and is a valid thing to do, but in my case I have a ton of unrelated existing code that I have to refactor just to correct sorting.

johnrom commented 3 years ago

You can just convert Formik's version back on submit by using reduce() instead of refactoring the whole project.

const backToObject = values.yourValue.reduce((obj, value) => {
  obj[value.key] = value;
  return obj;
}, {});
tameem-hamoui commented 3 years ago

that is kind of what im doing but there are many pieces to it and its getting tricky to convert the data back and forth. it would be nice if formik respected the order of javascript keys. at least give the user an option to make it re-render when the order of the keys change.

johnrom commented 3 years ago

I think this would be a breaking change especially in terms of enableReinitialize. I'm not totally against it but probably 40/60 against it. Formik's values object is very much intended to be a bag of nested properties in no specific order, and I believe many people, when updating an object field's value will do something like:

setFieldValue("myObjectValue", {
  ...prevMyObjectValue,
  myUpdatedProp: 'updated',
});

In order to make a change like this, we would have to determine that even though myObjectValue is an object, and could be considered to have nested fields, it is in fact NOT nested fields and is a single field which may or may not be sorted. Even then, some people might not expect performing a spread operation like the above to cause updates to values, and will create the exact opposite issue of this: "Formik triggering renders when order of keys in object changes :("

My recommendation is to use TypeScript, which will make it very clear when you need to convert back and forth, and to create a selector for converting back and forth, so even if you need to make this conversion 1000 times in your code, all you need to do is:

interface KeyedValue<Key extends string> { key: Key, value: string };
interface KeyedObject = {
  [Key: string]: KeyedValue<Key>,
}

const selectKeyedObjectFromKeyedValues = (
  values: KeyedValue<unknown>[]
) => values.reduce<KeyedObject>((keyedObject, keyedValue) => {
  // I don't know if TS supports this conversion yet, so you may need to cast this value.
  keyedObject[keyedValue.key] = keyedValue;
  return keyedObject;
}, {});

const selectKeyedValuesFromKeyedObject = (keyedObject: KeyedObject) => keyedObject.values();

// 1000 times? easy
const objectValue = selectKeyedObjectFromKeyedValues([ { key: 'abc', value: "1" } ]);
const arrayValue = selectKeyedValuesFromKeyedObject({ abc: { key: 'abc', value: "1" } } });

Now, TypeScript will yell at you whenever you forget to convert. If you can't use typescript, just create the selectors and pray you never forget. That's not a Formik issue, that's just the state of things until you migrate to strongly typed javascript.

tameem-hamoui commented 3 years ago

That is a good point. When we are saving values you'd have to make sure you are maintaining the same order in order to preserve the order of the fields.

My issue is that we are creating temp keys in the formik object in order to create new / or to-be-deleted table rows that are applied on save. When I convert the object into an array I lose those special keys. I wish I could add those special keys to the field values themselves but the structure of those are coming directly from the database and I'm not allowed to add more keys (like that special form key) to them myself. I think ultimately to solve this something with our data needs to change.

johnrom commented 3 years ago

When working inside of Formik (or any form library), it's values shouldn't be considered your database object. It is only the inputs and values necessary to construct your database object.

When handling the data in Formik, you can add the special key to the Value object for use in an array, and remove it when it is converted back to Keyed Object format. What is inside Formik doesn't have to be the same as long as it's converted before entering into Formik (like via initialValues, setValues), and converted back when passed back out (like via onSubmit). This is why I recommend TS, it makes this enforceable.

Once you have an array, you can use FieldArray to help manage adding to, deleting from, and sorting your values array.

https://formik.org/docs/api/fieldarray