microsoft/TypeScript

Unsafe array mutation allowed by loose generic constraints in item-creator functions

Closed this issue Ā· 10 comments

šŸ”Ž Search Terms

mutate array, covariance, covariant, invariance, invariant

šŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about covariance and invariance

āÆ Playground Link

https://www.typescriptlang.org/play/?target=10&jsx=0&ts=5.9.3#code/MYewdgzgLgBMBOBTAhlRBJNBbGBeAUDDADyFEwAiMiAHmmACYQwDeZ5MAlgwFwzTxOYAOYBudjAC+AGgkAVanUSNmbDkU7YIfCgG0AuuI4yyAPgAUEhCjQA5RAHdMiLH3MBKPKcqyin3KZk5sKCvDBy7nwKAawSSFAArvBgseowAHSZIdy+6pou2jC6menZDOn5WBDScEioiPZO2B76uUSSRlLi+FAAngAOiDAA4qF4qVxafGrk3HwCQmISYMhYiPNQgiKdkgbiHfj4oJCw1vXOWADyYKPc4x5u5tD1fLcM-t5vH2RnaBfmHi8MHMMw0YQA5GBHAB9bjg3KSdzucRAA

šŸ’» Code

I have a grid on which I want to create items by using a helper-function. This function should return a new version of the grid.

The type of the grid is as following:

type Grid = {
  items: {
    id: string;
    name: string;
  }[];
};

In the create-item function, I thought generics would be a cool idea, so I went for this definition:

const createItem =
  <
    D extends {
      id: string;
    },
    T extends {
      items: D[];
    },
  >(
    createNewItem: () => D,
  ) =>
  (grid: T): T => {
    return {
      ...grid,
      items: [...grid.items, createNewItem()],
    };
  };

But when invoking this function, I found a serious flaw in my types:

const createItemOnGrid = (): ((grid: Grid) => Grid) =>
  createItem(() => ({
    id: 'new_id',
  }));

šŸ™ Actual behavior

No error, but data that does not comply with the type when running the code.

šŸ™‚ Expected behavior

An error at compile time.

What gave me the expected result was to rewrite the function as following:

const createItem =
  <
    D extends {
      id: string;
    },
    T extends {
      items: D[];
    },
  >(
    createNewItem: () => T["items"][number],
  ) =>
  (grid: T): T => {
    return {
      ...grid,
      items: [...grid.items, createNewItem()],
    };
  };

Note that I only changed the definition of createNewItem here.

But with the code provided first, I would like to get an error when adding the element to the array, because the types could be incompatible (as shown).

Additional information about the issue

Someone in discord pointed me to the terms "covariance" and "invariance", and I started reading https://stackoverflow.com/questions/8481301/covariance-invariance-and-contravariance-explained-in-plain-english.

When searching for something in this direction with regards to TypeScript, I came to https://stackoverflow.com/questions/60905518/why-are-typescript-arrays-covariant and further via #1394 to #48240, but I couldn't find a way of getting a helpful result.

I also saw this in the documentation https://www.typescriptlang.org/docs/handbook/2/generics.html#variance-annotations but couldn't see why my approach still should not error out.

Discord thread, where I raised this issue first: https://discord.com/channels/508357248330760243/1430830666865709096

An error at compile time.

Can you clarify where and what the bug is in this sample? It doesn't manifest any errors running it so I'm not quite sure what you're getting at

jcalz commented

I'd say their problem is

createItemOnGrid()({items:[]}).items.forEach(i => i.name.toUpperCase())

I assume they're tripping over the fact that generic object spread is typed as an intersection which goes badly when properties are overwritten. Which means the most basic repro here looks like

function foo<T extends object>(x: T): T {
  return { ...x, a: 0 }
}
foo({a: "abc"}).a.toUpperCase()

and it's a duplicate of #42690?

I'd say their problem is

createItemOnGrid()({items:[]}).items.forEach(i => i.name.toUpperCase())

Yes, that's my main problem.

Can you clarify where and what the bug is in this sample? It doesn't manifest any errors running it so I'm not quite sure what you're getting at

Sure, I'll do my best:

When writing the function, I assumed, that D could just be one specific type, means, that the returned value of createNewItem must fit T['item'][number], or the return-type of the function would be invalid when calling, because createItem does not return the same type it gets as input if the return-value of the createNewItem function does not return something compatible with T['item'][number].

This said, I thought, there could either be an error at the return-statement, because it's not ensured that the returned value and the expected return value are compatible - effectively making it a duplicate of #42690, or there could be an error when calling the function, because the argument given as function is not compatible with T["items"][number]. The returned value of the function would (rather than it being T) be something in the realm of T & { items: D[] } but no longer just satisfiable by T

The latter is what happens if I remove the explicit typing of the createItem function, which begs the question, why I otherwise do not get an error like

'T' could be instantiated with a different subtype of constraint - ts(2345)

For the last two paragraphs, I don't think it is a duplicate of #42690 nevertheless šŸ˜…

Btw: Even if I type it as such, it doesn't error - and I guess here is where I would expect the error:

type Grid = {
  items: {
    id: string;
    name: string;
  }[];
};

type NewGrid = Grid & {
    items: {
        id: string;
    }[];
}

// These two function-definitions should error IMO.
const testFn1: (a: NewGrid) => Grid = (a) => a
const testFn2 = (g: NewGrid) => g.items.map(el => el.name)
jcalz commented

I don't understand why those should error (although intersections of arrays are weird). You'll find that both testFn1 and testFn2 will reject an input like { items: [{ id: "abc" }] }. If intersections-of-arrays were automatically converted to arrays-of-intersections (and they're not, see #41874) then NewGrid would be exactly the same type as Grid. I don't see NewGrid type itself as the real problem. NewGrid should definitely be assignable to Grid.

The problem is that { ...grid, items: [...grid.items, createNewItem()], } does not actually result in a NewGrid, even though that's the inferred type of createItem<{ id: string }, Grid>(() => ({ id: 'new_id' })). Spreading is not intersection unless there's no overlap in properties. As soon as you have overwritten properties, then spreading {...T, ...U} starts looking like Omit<T, keyof U> & U (except that's not quite right for optional properties of U and ugh). Intersections have the advantage of being simple, and good enough for lots of use cases. But when it goes wrong it really goes wrong. This still looks like #42690 to me, and unless we get something like #10727, I imagine it will persist.

wgrea commented

The generic signature implies type preservation, but spread breaks that promise. It’s not just a variance issue—it’s a trust issue between inferred return type and actual mutation.

Typing createNewItem as () => D decouples it from the array’s true element type. That opens the door to structurally incompatible mutations—breaking the integrity of T.

If you mutate a generic array, your mutation source must be typed as T["items"][number]—not just a compatible-looking subtype.

Spread isn’t additive—it’s destructive. You’re not extending T, you’re replacing parts of it. TypeScript treats the result as T, but it’s really a Frankenstein type.

This feels like a missing constraint in the type system: array mutations inside generic structures should preserve element type integrity. Otherwise, the return type is unsound—and dangerously misleading.

Why do people keep saying "mutation" here? Spread is not mutation

jcalz commented

Maybe they mean it in the sense that {...["a", "b", "c"]} is a mutant? /s

wgrea commented

Fair point—spread isn’t mutation in the runtime sense.

I used ā€œmutationā€ to describe the structural shift in type integrity: the array’s shape changes, but TypeScript still treats it as preserved.

It’s less about runtime mutation and more about type trust erosion—where the inferred return type promises safety, but the actual structure breaks it.

Happy to reframe if there’s a better term for that kind of type-level misalignment.

The linked duplicate(s) are the real root cause here - a lack of a proper higher-order spread operator is why intersection is used instead, and intersection has some baked-in assumptions about linearity that get violated in this example.

// These two function-definitions should error IMO.
const testFn1: (a: NewGrid) => Grid = (a) => a
const testFn2 = (g: NewGrid) => g.items.map(el => el.name)

This fundamentally violates a core assumption of intersection types -- that T & U is always assignable to T. This is why it's always a super bad idea to intersect two arrays (and why generic intersection is unfortunately so fraught).

This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes.