enonic / lib-util

Apache License 2.0
3 stars 5 forks source link

data.forceArray applied on undefined #28

Open gbbirkisson opened 5 years ago

gbbirkisson commented 5 years ago

In my opinion data.forceArray should return an empty array when passed undefined. i.e:

forceArray(undefined) -> []

not

forceArray(undefined) -> [undefined]

hjelmevold commented 5 years ago

I agree that this would make things easier, and the same goes for null

But I'm not sure if it's best to return [] or if it's better to return the original undefined (or in case of null: null)

Is there a good argument for why undefined or null should return an empty array? I have no preference.

Any suggestions, @ComLock ? @poi33 ?

poi33 commented 5 years ago

I don't see the use for undefined -> [undefined] and null -> [null] So i would defently want them to return null or undefined in those cases. I could see them returning an emtpy array [], but this could mask/hide errors in the code, so i prefer them retruning the value instead.

ComLock commented 5 years ago

I agree with @gbbirkisson about undefined

forceArray(undefined) -> []

Null however is an actual value so

forceArray(null) -> [null]

Implememting this with array.filter(x => x) would be a bad idea because that would alter an array in which the number of elements is important.

[1, 0, -1, 'str', '', null, undefined, NaN, Infinity, () => {}]
poi33 commented 5 years ago

Okay can someone add the force array with undefined returning an emtpy array?

gbbirkisson commented 5 years ago

I think ForceArray should always return an array. You should be able to do:

if (content.data) {

  var maybeNull = content.data.someInput;
  forceArray(maybeNull).forEach(function (item, index) {
     // I wont get item == null
  });

}

ForceArray would then cover all input type occurrences, i.e:

ComLock commented 5 years ago

In JS undefined means a variable has been declared but has not yet been assigned a value. null is an assignment value. It can be assigned to a variable as a representation of no value.

Related thoughts:

forceArray is mostly used for working with node and content data.

Enonic XP does not store undefined or null. Enonic XP stores an array with a single item as just the item. I don't know what happens to an array with just a single null value in it.

But either way null is an actual value in js.

ComLock commented 5 years ago
let variable = true;

// I believe this
variable = undefined;

// Is the same as
delete variable;

// While
let anotherVariable = null;

// Is NOT the same as
delete anotherVariable;
rymsha commented 5 years ago

Seems like forceArray(v) behaves very much like [].concat(v) .

rymsha commented 5 years ago

One doesn't need forceArray to get what @gbbirkisson specified either. [].concat(undefined || []) -> [] [].concat('a' || []) -> ['a'] [].concat(['a'] || []) -> ['a']

GlennRicaud commented 5 years ago

Actually the function almost works like concat today (with the exception of no parameter passed I guess).

But as this library says: Functionality to help with handling returned data from XP (json)
A Field on contents/nodes can be: (1) an array of values, (2) the value, or (3) undefined. The purpose of this function is I think that we would like an array of the values in all cases. So that it returns (1) the array, (2) an array of the value, or (3) an empty array

The null is more arguable.