I have some code that filters an array with type (Suggestion|undefined)[] into an array with type Suggestion[], but I feel it does so very inelegantly:

const optionsProvider: AutocompletionProvider = () => {
    const results: (Suggestion|undefined)[] = flagDescriptions.map(descriptions => {
        let shortFlagWithArgument = descriptions[0].match(/^ *-(\w) (\w*)$/);
        let shortFlagWithoutArgument = descriptions[0].match(/^ *-(\w) *(.*)$/);
        if (shortFlagWithArgument) {
            const flag = shortFlagWithArgument[1];
            const argument = shortFlagWithArgument[2];
            const description = combineManPageLines(descriptions.slice(1));

            return new Suggestion({
                value: `-${flag}`,
                style: styles.option,
                description,
                displayValue: `-${flag} ${argument}`,
            });
        } else if (shortFlagWithoutArgument) {
            const flag = shortFlagWithoutArgument[1];
            const description = combineManPageLines([shortFlagWithoutArgument[2], ...descriptions.slice(1)]);

            return new Suggestion({
                value: `-${flag}`,
                style: styles.option,
                description,
            });
        }
    });
    const filteredResults: Suggestion[] = [];
    results.forEach(result => {
        if (result) {
            filteredResults.push(result);
        }
    });
    return filteredResults;
};

In straight JS, this would just be array.filter(element => element !== undefined) but the best working solution I could come up with in TypeScript was to push the defined elements into a separate array within a forEach.

For context:

type AutocompletionProvider = (context: AutocompletionContext) => Suggestion[] | Promise<Suggestion[]>;

Original PR: https://github.com/shockone/black-screen/pull/623/files

  • We allow links in questions, but the code you wish to be reviewed must be in the question body itself. – Dan Pantry Jul 20 '16 at 6:57
  • 1
    Sorry @DanPantry I didn't realize. Fixed. – Drew Jul 20 '16 at 7:06
  • I don't understand the point of the type (Suggestion|undefined)[] in the first place. It is ok to put undefined into a Suggestion[], so why not change the type to be simply Suggestion[] throughout? – Daniel Ryan Jul 30 '16 at 3:10
up vote 8 down vote accepted

To be using strict null checks, you must be on "typescript": "^2.0.0". Thus, my example will be using this version of typescript with "compilerOptions":{"strictNullChecks": true}.

It does seem that the filter solution is the most elegant solution to the titular question. Sadly, it seems that the typescript static analysis is unable to track this behavior:

const source: (string|undefined)[] = ["one", "two", "three", undefined, "five"]
const filtered: string[] = source.filter(element => element !== undefined)
//  [ts]
//    Type '(string | undefined)[]' is not assignable to type 'string[]'.
//      Type 'string | undefined' is not assignable to type 'string'.
//        Type 'undefined' is not assignable to type 'string'.

This seems to be the case because of the declaration of the filter method:

/** Returns the elements of an array that meet the condition specified in a callback function. */
(method) Array<string | undefined>.filter(callbackfn: (value: string | undefined, index: number, array: (string | undefined)[]) => any, thisArg?: any): (string | undefined)[]

In this case, our own type logic beats out the compiler's. It's these cases that is the reason that Type Assertion is part of the language spec. Thus:

const source: (string|undefined)[] = ["one", "two", "three", undefined, "five"]
const filtered: string[] = source.filter(element => element !== undefined) as string[]

This transpiles exactly as you would expect -- in this case, transpiling is as simple as just removing the type information. If you need to convince the language to convert two unrelated types, you need to step through any to tell the compiler to back off: object as any as string.


Now, looking at the rest of the code:

shortFlagWithArgument and shortFlagWithoutArgument can be consts, as you do not mutate their values.

Personally, I would add an explicit final return to the flagDescriptions.map operation. This makes explicit why the undefined needs to be in the results array in the first place.

The regex patterns seem good candidates for refactoring into a reusable constant. Depending on your project ecosystem, however, it may be better just to leave them inlined.

Typescript 1.6 introduced user defined type guards that are a function whose return type is a boolean that is cast as a type predicate. To define a simple isString type guard (note the X is string return type), use syntax like below:

function isString(str: string | undefined): str is string { return str !== undefined }

or as a lambda with a slightly different check:

const isString = (str: string | undefined): str is string => !!str

Passing a type guard function as the array filter callback predicate function will both filter the array and allow Typescript to correctly infer the return type. To expand on the accepted answer's example:

const source: (string|undefined)[] = ["one", "two", "three", undefined, "five"]
const filtered: string[] = source.filter( isString )

The isString lambda can also be expanded inline

const filtered: string[] = source.filter( (str: string | undefined): str is string => !!str )

This approach allows the Typescript compiler to infer types instead of explicit type casting. Yes, the type inference is dependent on the predicate function being correctly defined. But an incorrect predicate will give the wrong answer when filtering with a type cast too.

  • This is a good use of new language features, and I learned something! +1 – CAD97 Feb 24 at 23:51
  • This is a beautiful solution, thank you. – SimplGy Sep 18 at 1:13

Building on @RichardJohnson's answer: why not write one function that can filter any array of possibly-undefined values? Let's make it generic.

export function filterUndef<T>(ts: (T | undefined)[]): T[] {
  return ts.filter((t: T | undefined): t is T => !!t)
}

This simplifies your filter call, too:

const nums = [20, 44, undefined]
const numsSafe = filterUndef(ages)

The typescript compiler can even use type inference to figure out numsSafe is number[]. Just define this function once, and reuse it whenever you want to strip the undefineds from an array in a typesafe way.

  • I added a link to the answer - I presume you meant Richard's answer but if I got that wrong then please updated it appropriately. – Sᴀᴍ Onᴇᴌᴀ Aug 24 at 23:20

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.