Dealing with a union type where one of two functions can provide the value-CodePudding

I have two methods that are very similar and I want to DRY up the repeated code. The separate functions are:

//returns a single element
const getByDataTest = (
  container: HTMLElement,
  dataTest: string
) => {
  const element = queryByAttribute(
    'data-test',
    container,
    dataTest
  );

  if (!element) {
    throw queryHelpers.getElementError(
      `Unable to find element by: [data-test="${dataTest}"]`,
      container
    );
  }
  return element;
};

//returns multiple elements
const getAllByDataTest = (
  container: HTMLElement,
  dataTest: string
) => {
  const elements = queryAllByAttribute(
    'data-test',
    container,
    dataTest
  );

  if (elements.length === 0) {
    throw queryHelpers.getElementError(
      `Unable to find any elements by: [data-test="${dataTest}"]`,
      container
    );
  }
  return elements;
};

What I'm trying to do is merge these into one function that takes a multiple argument and uses either one query method or the other:

const getDataTest = (
  container: HTMLElement,
  dataTest: string,
  multiple = false
) => {
  //choose which query method to use
  const queryMethod = multiple ? queryHelpers.queryAllByAttribute : queryHelpers.queryByAttribute;
  const result = queryMethod(
    'data-test',
    container,
    dataTest
  );

  if ((multiple && result.length === 0) || !result) {
    throw queryHelpers.getElementError(
      `Unable to find any element by: [data-test="${dataTest}"]`,
      container
    );
  }
  return result;
};

Because result is HTMLElement | HTMLElement[], the result.length complains. I have tried to extend the condition to if ((multiple && result.length && result.length === 0) but it still complains at the first instance of result.length.

CodePudding user response：

Not sure if this is the best way to do it, but you can use User Defined Type Guard to narrow the type. I have tried to fake the example you have given.

function isArray(result: any): result is HTMLElement[] {
    return 'length' in result;
}

const getDataTest = (
  container: HTMLElement,
  dataTest: string,
  multiple = false
) => {
  //choose which query method to use
  const queryMethod = multiple ? queryHelpers.queryAllByAttribute : queryHelpers.queryByAttribute;
  const result = queryMethod(
    'data-test',
    container,
    dataTest
  );

  if ((isArray(result) &&  result.length === 0 ) || !result) {
    throw new Error(
      `Unable to find any element by: [data-test="${dataTest}"]`,
      
    );
  }
  return result;
};

I have tried to emulate it, see the TS Playground: https://tsplay.dev/N535oW

Update after reading @jcalz comment. No need to jump through hoops as I suggested above (facepalming myself!), you may just get away with one minor change:

Use

  if ((Array.isArray(result) &&  result.length === 0 ) || !result) {

instead of

  if ((multiple && result.length === 0) || !result) {

CodePudding user response：

TypeScript does not let you index into a value of a union type with a key if that key does not exist on all members of the union:

let result: HTMLElement | HTMLElement[];
result = Math.random() < 0.5 ? document.body : [document.body];

result.length // sad

The reason for that is that object types are open/extendible, so nothing prevents an HTMLElement from having a length property of some unknown value type:

const possibleResult = Object.assign(document.createElement("div"), { length: "whaaa" });
result = possibleResult; // <-- this is fine

console.log(possibleResult.length) // "whaaa", not a number

So technically you cannot assume that there is a length property and that if there is one it will be a number or if it is a number that you've got an array instead of a weird HTMLElement-with-extra-length property.

The "right" way to do this in TypeScript is to use a type guard to narrow result to either HTMLElement or HTMLElement[]. TypeScript lets you use Array.isArray() as such a type guard, so the fix could be like this:

if (Array.isArray(result)) {
  result.map(x => x.tagName) // happy
} else {
  result.tagName // also happy
}

Another thing you could do is use the in operator as a type guard:

if ("length" in result) {
  result.map(x => x.tagName) // still happy
} else {
  result.tagName // ditto
}

TypeScript uses "length" in result to filter the type of result to those union elements with or without a known length property. This is very convenient... but has exactly the same problem that result.length with no check has: it is of course possible that "length" in result returns true without result being an array at all. So the in operator allows unsound things to happen. But this is intentional, (see this comment on microsoft/TypeScript#39065); the TypeScript team thinks that someone writing result.length might not necessarily be trying to discriminate a union based on this, while someone writing "length" in result is definitely trying to do so and so the language should let them. There is at least one feature request at microsoft/TypeScript#45211 saying that instead of just yelling at you for writing result.length, the compiler should maybe say "if you're trying to narrow result based on the presence of the length property, try "length" in result".

There are other ways to narrow unions, but one of those should work for you.

Playground link to code