Destructure with care. The story of a Typescript gotcha

A problem that even the strictest Typescript/ESLint settings cannot help with.

The decision to inline types directly into method signatures vs extracting into an interface or type is a common discussion point at the moment.

The inline vs interface vs type debate very much focuses on the RHS (right-hand-side) of the parameter definition - but today I'd like us to consider a dangerous pattern on the LHF side instead.

First though, a quick refresher on those terms:

Types inline

function matches({ element, xpath }: { element: HTMLElement, xpath: string}): boolean {}

Separate Interface

interface Props {
    element: HTMLElement;
    xpath: string;
}
function matches({ element, xpath }: Props): boolean {}

Separate Type

type Props = {
    element: HTMLElement;
    xpath: string;
}
function matches({ element, xpath }: Props): boolean {}

And by RHS/LHS I'm referring to the sides separated by the :

function matches({ element, xpath }: Props): boolean {
                 ^^^^^^^^^^^^^^^^^^  ^^^^^
                 LHS                 RHS
}

With that clarification out of the way, let get into it.

The first implementation

Our example is a function that just takes a single parameter, and applies destructuring inline.

Both element and xpath are required properties on the first parameter to this function, so callers must provide them both.

interface Props {
    element: HTMLElement;
    xpath: string
}

function matches({ element, xpath }: Props): boolean {
  return document
    .evaluate(xpath, element, null, XPathResult.BOOLEAN_TYPE, null)
    .booleanValue;
}
  • Note how the first and only parameter here is a destructuring assignment. It's often thought of as 'unpacking' the element and xpath properties, but as we'll see later I don't find this framing useful.

Now, before we start to find bugs, let's quickly break down what parameter position destructuring assignments actually represent "behind the scenes".

If we remove it from the signature and re-create the functionality directly in the function body it helps to reveal some hidden elements

interface Props {
    element: HTMLElement;
    xpath: string
}

function matches(): boolean {
  let { element, xpath }: Props = arguments[0];
  return document
    .evaluate(xpath, element, null, XPathResult.BOOLEAN_TYPE, null)
    .booleanValue;
}
  • We've removed everything from the function signature for clarity
  • We've introduced let to make it crystal clear that destructuring is an assignment. That is, we're creating 2 new bindings in this function's scope, element and xpath

We've kept type declaration : Props in place, to mimic what was in the signature, and arguments[0] just gives access to the first parameter.

const { element, xpath }: Props = arguments[0];
  • You can read this line now as
    • create new bindings element and xpath with types that match the corresponding types in Props
    • for example, binding element has type Props['element'], and xpath has type Props['xpath]

Nothing too exciting or surprising here. Typescript will do it's job and the new bindings can be used safely.

It all changes with defaults, though

Let's evolve our function to now also support regular CSS selectors. We'll change xpath to selector, and because xpath and css selectors are not interchangeable, we want a new property to help differentiate between them.

We'll call that new property kind, and we'll use a union of 2 literal types "xpath" | "css" as a poor man's enum.

More importantly though, because we already have a bunch of consumers of this function that we don't want to change, we'll introduce a default value for kind and make it optional.

interface Props {
    element: HTMLElement;
-   xpath: string
+   selector: string
+   kind?: 'xpath' | 'css'
}

-function matches({ element, xpath }: Props): boolean {
+function matches({ element, selector, kind = "xpath" }: Props): boolean {

}
  • Note: This is still perfectly type-safe ✅. Typescript will verify that any default value provided for kind has to be assignable to Props['kind'], which expands to "xpath" | "css" in our case.

Let's see what happens if we try to assign a different default value - something other than "xpath" | "css". (broken onto multiple lines for clarity):

function matches({
    element,
    xpath,
    kind = "nope!"
           ^^^^^^^
Type "nope!" is not assignable to type "xpath" | "css"
}: Props): boolean {

}
  • ✅ Nice! This is what we want - if we provide a default value within the destructuring like this, we want to ensure it can only be a value assignable to the expected type. "nope!" fails that test, so Typescript prevents the bug!

The next evolution, cleaning up the function signature.

As time goes on, more params are added and someone decides to drop the destructing from the function signature and instead do it as the first statement in the function body

interface Props {
    element: HTMLElement;
    xpath: string
    kind?: 'css' | 'xpath'
}

function matches(props: Props): boolean {
    const {
        element,
        xpath,
        kind = "xpath"
    } = props;

    // snip
}
  • Ahhh! a breath of fresh air - having just the parameter name props alongside it's type Props leave the function signature in a great place. Docs can be added to the interface/type and the amount of values can grow at will without becoming difficult to read/reason about
  • We've all seen crazy destructuring patterns that end up hogging the screen space in the function signatures - so why wouldn't you do this?

Well, just wait (and hold my 🍻)...

There's a 'trap' you can fall into when combining types, destructuring and default values.

I've used Typescript since it was released, and have fallen for this on numerous occasions!

interface Props {
    element: HTMLElement;
    xpath: string
    kind?: 'css' | 'xpath'
}

function matches(props: Props): boolean {
    const {
        element,
        xpath,
-       kind = "xpath"
+       kind = "this will not error!"
    } = props;
}
  • 😱😱😱 This change does NOT cause a Typescript error!
  • We only wanted to allow "css" | "xpath", but "this will not error!" is being allowed here!
  • This affects more than just literal types - but this a good example since so many programming patterns in Typescript make heavy use of unions, and narrowing them.
  • Now, depending on how you go on to use kind, it may or may not be a big bug in your program. But the simple fact that Typescript cannot prevent you setting an incorrect default value in this way concerns me!

👀 So, even though props is marked as type Props, the type-safety doesn't seem to follow though to the props.kind property - why?

Lets de-sugar the destructuring - it might help us understand how this hole in type-safety comes around.

interface Props {
    element: HTMLElement;
    selector: string
    kind?: 'css' | 'xpath'
}

function matches(props: Props): boolean {
    const element = props.element;
    const selector = props.selector;
    const kind = props.kind ?? "foooo";
}
  • This is a good-enough approximation of what the previous destructuring was doing.
  • This highlighted line is now effectively saying:
    • create a new binding kind
    • with the type: Props['kind'] | "foooo"
    • which expands out to "xpath" | "css" | "foooo"

And that explains why Typescript won't error here - the new binding kind is just that - a new binding, it's not required to have overlap with Props['kind'] - it's just that Props['kind'] contributes a type that kind could be assigned to.

So we've essentially disconnected props.kind from the local binding kind. They have a loose relationship, but not the kind that will help us write correct programs. 😭

If we wanted to maintain the type-safety with this pattern, (still using this long-hand demo), we'd have to apply type declarations for each property 🤮

interface Props {
    element: HTMLElement;
    selector: string
    kind?: 'css' | 'xpath'
}

function matches(props: Props): boolean {
    const element = props.element;
    const selector = props.selector;
-   const kind = props.kind ?? "foooo";
+   const kind: Props['kind'] = props.kind ?? "foooo";
}
  • ✅ By applying that type declaration, const kind: Props['kind'], Typescript will check all possible values in the assignment and error if a type is invalid. This means "fooo" is now rejected.
  • The same thing applies for the other properties too, omitted here.

Also, if we wanted to apply the same 'fix' to the destructuring version, it would look like this:

interface Props {
    element: HTMLElement;
    xpath: string
    kind: 'xpath' | 'css'
}

function matches(props: Props): boolean {
    const {
        element,
        xpath,
        kind = "foooo"
-    } = props;
+    }: Props = props;
}
  • Here we're re-adding the full type declaration back in, directly after the closing } from the destructuring, making it safe again.

This whole thing is becoming rather unsatisfactory though - it's so easy to forget to add these declarations, and no amount of strict Typescript or ESLint settings can help us here 😭

Summary:

This is not just limited to function signatures - that's a really common case, but the following snippet is equally dangerous/error-prone.

interface Props {
    xpath: string
    kind?: 'xpath' | 'css'
}

const props: Props = { kind: "css", xpath: "..." };

const {
    kind = "a"
} = props;
  • 🧨 As before, Typescript cannot help you here - this 'type mistake' can be perfectly explained with the long-hand de-sugaring as shown above.
  • Remember this line translates to: 'create a new binding kind with type "xpath" | "css" | "a"' - which is not what you're probably intending.

This is not a 'bug' in Typescript, or a problem with JavaScript really - I just view it as an easy mistake to make, something to watch out for.

Going forward I'm personally going to use the inline-destructing pattern more - even though I don't like noisy function signatures. The ability to prevent an entire class of type-safety bugs is just too appealing.

Regardless of which strategy you choose, having a better understanding of the semantics of destructuring assignments could help you identify/fix bugs sooner 🤞

Finally, the next time you encounter people discussing what happens on the RHS of that :, remember that the LHS is far more important for program correctness, so don't leave it out of the conversation! 👋