Flatten does not flatten empty objects or arrays

[lucas-rudd]

Issue:

Suppose I have the following object

{ "foo": { "bar": "hello-world" }, "data": {}, "tests": [] }

I would expect this object to be flattened if I need it for some sort of operation which does not support complex objects.
Perhaps it would be flattened in this way
{ "foo.bar": "hello-world", "data": "{}", "tests": "[]" }

Or, perhaps there is a better way to do this; but, what actually occurs is the following

{ "foo.bar": "hello-world", "data": {}, "tests": [] }

Where, in reality, this has not been flattened. There still exists nested objects, they are just empty.

[aecepoglu]

If you have a sensible proposition for how empty arrays and objects should be treated, I'll quickly fix this for you.

[lucas-rudd]

Sure... I don't know if this is sensible, and I understand the complexity in this issue, but what about something like

{
    a: '[Object|Array]',
    a1.a11: 1,
    b: "test",
    c: 12,
}

and

{
    a: '[Object|Object]',
    b: "test",
    c: 12,
}

to indicate that we have an empty object/array.

I understand this is an odd case, because it requires some semi-unique value to indicate that the unflattened object should contain an empty nested object/array, rather than interpreting the string literally. In which case, if your original object had a literal string which was '[Object|Array],' for whatever reason, it would get converted to an array in the unflattening process, which is not what we would want.

[aecepoglu]

I feel the flattening operation should refrain from modifying the values. Maybe removing the key that points to the empty object is more desirable. This behaviour could be controlled via an option.

[edosssa]

@aecepoglu I don't quite agree. The whole aim of flattening is just to well, flatten, and not modify the object graph in the process i.e all keys present before flattening must be present after flattening. Also, the developer may have some library that consumes the new object and requires that the keys (empty or not) be present. IMHO, I feel using a string to mark an empty key is a much better approach.

[lucas-rudd]

The only problem I have with removing the key altogether is similar to what @King-kay mentioned; an empty object is a 'truthy' value in javascript, whereas a null value is not. So, if I were doing something like

if ( object.key )
...

Then if key is an empty object, this statement would evaluate to true. Removing it causes this to evaluate to false.

Similarly, modifying something like this

{
  'a': {
    'b': { }
  }
}

To this

{
  'a.b.': ''
}

(where the key is a dot b dot to indicate an empty object)
produces the same issue mentioned above (an empty string is falsey)

Also, removing the key is irreversible, whereas modifying the value is not.

[aecepoglu]

My 2 points to make are:

1. unflatten(flatten({a: {}})) is ambiguous when you use a special token
2. Altering the values does alter the value of what that object represents. Imagine using the output of flatten() as a mongodb query.

[edosssa]

This is clearly an edge case and people have different ways of dealing with edge cases - so why not let them?

I think we should let the user specify a callback (maybe in the options object) which is called whenever an empty key is encountered during flattening. The callback could then return a value which is assigned to the key. That way, the user is in complete control over what happens.

As a plus, we could also add default callbacks for standard stuff like: Preserve (leaves the empty object or array as the value. This already it's default behavior ) , Remove (removes the key entirely) e.t.c.

[aecepoglu]

Okay, the processor-function idea is pretty good IMO. Lets have a generic callback function that is called for each key-value pair encountered so the user can not only deal with {} values but also any other key/value that needs tweaking.

It could work just like the JSON.stringify replacer https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify

flatten({
  a: {
    b: "c",
    d: {
      e: "f"
    }
  },
  g: 123,
  h: {},
  j: {}
}, {
  replacer: function (childKey, childValue, parentKey) {
    if (childValue === {}) {
      if (childKey == "h") {
        return undefined;
      } else {
        return "AN EMPTY OBJECT";
      }
    } else if (childKey == "d") {
      return "DEDUCTED";
    } else {
      return childValue;
    }
  }
});

/* would produce:
{
  "a.b": "c",
  "a.d": "DEDUCTED",
  "g": 123,
  "j": "AN EMPTY OBJECT",
}
*/

[edosssa]

Looks good! @lucas-rudd what do you think?

@aecepoglu This should definitely be included in the next release. Are you going to do a PR?

[lucas-rudd]

I think that's a brilliant idea!

Unflattening is still an issue. However, I think anyway you look at it that will be an issue in this case.
Could we also include a callback for unflattening as well?

So that in the unflattening process if I encounter 'AN EMPTY OBJECT' I will know what to replace it with.

[edosssa]

A ha! I had the exact same thoughts.

It's clear that every callback registered to mutate the object while it's flattening should have an "opposite", which reverses it during the unflattening process. Otherwise, we would have an object that when flattened and unflattened does not equal the original object.

I think we could use a "Resolver" object that contains two functions, one that's called during flattening and the other during unflattening. That object could then be passed as an option. We could even add a check to see if the resolver has both callbacks present else it should throw an error (not sure how sensible that would be but oh well).

[lucas-rudd]

@King-kay could you elaborate a tad? I'm having trouble understanding how that would look.

I also think that a default callback should be provided by the framework in the form of this flag.

As it stands, when the user calls flatten they expect their object to be flat. Meaning that it is a single layer deep. Currently, this library does not do that. I think it would be better to provide some default callback to either remove empty objects, or to set them to undefined, or something.

It's better to take some action and actually have a flat object than to take no action and have a complex object after I supposedly flattened it.

[aecepoglu]

I have did the flattening bit but the unflatten code is a tad bit uglier so it'll take some more time.

And still @lucas-rudd , I'm in favour of removing empty objects since they represent nothing logically. Because different people will have different behaviours in mind when they want to flatten something which can't be flattened without being presumptuous, I want it to be the callback way.

[lucas-rudd]

@aecepoglu let me know if you need any help or if I can contribute in any way. I would love to get this knocked out sooner rather than later.

[aecepoglu]

@lucas-rudd my branch is at https://github.com/aecepoglu/flat . The feature is added to flattening, but not unflattening. I have written tests for both cases; so if you can send me a PR which makes both of them pass we can be done with this.

[kylecordes]

Here's an example case for this issue comes up: some downstream code simply does not tolerate nested objects. So the developer (me in this case) goes looking for a library/function to flatten objects and make this not happen. This library is 99% of the way there, aside from this quite surprising case where some objects sail right through, un-flattened, still containing nested objects causing downstream trouble.

Reversibility is not the only important criteria - in some use cases it is less important than "the output is always flat no matter the input".

[yelhouti]

@lucas-rudd , thr PR by @aecepoglu looks great to me and is consitent with how stringify wokrs, could you merge it please?

[lucas-rudd]

@yelhouti I do not see that PR. Plus, I am not the owner of this repository and do not have merge privileges :( sorry to disappoint.

[aecepoglu]

hey, I got bored of the PR making no progress and closed it. the code is still there. our projects are dependant on my fork so the code is there to stay. You're welcome to use the fork, open prs for other awaiting issues, etc

http://github.com/aecepoglu/flat

[yelhouti]

@timoxley can we use your help here please?