Add skipEmptyString option

[mum-never-proud]

Fixes #244

[sindresorhus]

This utility has no value over just checking directly.

[mum-never-proud]

done

[sindresorhus]

You also need to update index.d.ts. That one and the readme should be in sync. I would also like to see more tests.

[sindresorhus]

Don't do unrelated changes. I prefer this to stay a for-of loop instead of reduce, for readability.

[mum-never-proud]

I didn't refactor, rather a related change

if (options.skipNull) {
  for (const key of Object.keys(objectCopy)) {
    if (objectCopy[key] === undefined || objectCopy[key] === null) {
      delete objectCopy[key];
    }
  }
}

the same can be achieved with

if (options.skipNull || options. skipEmptyString) {
  for (const key of Object.keys(objectCopy)) {
    if (objectCopy[key] === undefined || objectCopy[key] === null || objectCopy[key] === '') {
      delete objectCopy[key];
    }
  }
}

suppose in future if we introduce other filters the list will go on...

is there any better way than this?

[sindresorhus]

is there any better way than this?

Do the same as now, but instead of reduce, use for-of loop, meaning no Object.asssign. You could maybe move the filtering logic here into a helper function (in this scope, not top-level).

[mum-never-proud]

hmm ok, if I remove the reduce this is how the code is (existing) will this work? sorry if I am not getting your point

const objectCopy = Object.assign({}, object);

for (const key of Object.keys(object)) {
    if ((options.skipNull && isNullOrUndefined(object[key])) ||
        (options.skipEmptyString && object[key] === '')) {
        delete objectCopy[key];
    }
}

this is another approach

const shouldFilter = key => (
    (options.skipNull && isNullOrUndefined(object[key])) ||
    (options.skipEmptyString && object[key] === '')
);

const formatter = encoderForArrayFormat(options);

const objectCopy = {};

for (const key of Object.keys(object)) {
    if (!shouldFilter(key)) {
        objectCopy[key] = object[key];
    }
}

we can add as many filters we need in future

your thoughts?

[sindresorhus]

this is another approach

^ The last code block here is what I had in mind. 👌 Sorry for being unclear.