store icon indicating copy to clipboard operation
store copied to clipboard

❓[SUPPORT]: removeItem repairType object possibly undefined

Open developer239 opened this issue 5 years ago • 1 comments

When I use removeItem state operator then TypeScript complains that message object can be possibly undefined even though I specified the type for removeItem operator. I could fix the issue with optional chaining but that is not possible if I want to use a utility library like Ramda

image

With Ramda:

image

How do people use state operators? Do they really use optional chaining EVERY time they use state operator? 🙂

developer239 avatar Jan 08 '21 14:01 developer239

After looking at the codebase I would summarize the problem like this:

When I am using removeItem I want it to do two things:

  • find a relevant item that matches my selectors
  • remove relevant item

Making the value in Predicate type optional forces me to do a third thing:

  • consider what to do with items that are not defined if there are undefined items in the array

I don't think that selector should be responsible for the third step.

// packages/store/operators/src/remove-item.ts

export function removeItem<T>(
  selector: number | Predicate<T>
): StateOperator<RepairType<T>[]> { // **existing is NEVER undefined**
  return function removeItemOperator(existing: Readonly<RepairType<T>[]>): RepairType<T>[] {
    let index = -1;

    if (isPredicate(selector)) {
      // **selector might fail here in some cases if there are undefined items in the array but we could filter the array before using the selector.** Selector should receive only items that it was designed to select from (in this case `T`)
      index = existing.findIndex(selector);
    } else if (isNumber(selector)) {
      index = selector;
    }

    if (invalidIndex(index)) {
      return existing as RepairType<T>[];
    }

    const clone = existing.slice();
    clone.splice(index, 1);
    return clone;
  };
}

I can create PR with a fix but I am not sure how would that affect the rest of the codebase but I am pretty sure that this will be too big of a breaking change to go through. I decided to create my own selectors instead.

developer239 avatar Jan 08 '21 16:01 developer239

Great news! v3.8.0 has been released and it includes some additional types exposed for building operators. We are closing this issue, because the improvements may assist you in resolving this issue. Please re-open it if the issue is not resolved.

Please leave a comment in the v3.8.0 release discussion if you come across any regressions with respect to your issue.

markwhitfeld avatar Mar 29 '23 14:03 markwhitfeld

@markwhitfeld Thank you 🚀

developer239 avatar Mar 29 '23 17:03 developer239