Bug: [consistent-indexed-object-style] shouldn't suggest `Record` when extending an another type about typescript-eslint HOT 7 OPEN

I'm surprised this has never come up before! Sometimes I just need to be able to access properties by index. This particular case is for some custom error logging where I want to grab all properties off of an incoming error, but exclude a few by name. TS complained at me for not having an index signature, so I added this. Here's my real world use case for the type I ran into this on

export interface IIndexableAxiosError extends AxiosError {
  [key: string]: unknown;
}

from typescript-eslint.

ok, fair enough. I've updated my code to grab the value using let val: unknown = error[key as keyof AxiosError]; which does feel a bit safer to me than what I was doing before. So thanks for that.

However, given my original code it still feels kinda wrong to recommend that an extended interface with an added index be recommended to use as as Record instead. That recommendation doesn't feel right to me since it can't really be done. Perhaps for this unique situation it could have a different error message?

from typescript-eslint.

Hmmm, the comments in this PR tell an interesting story for this case: #3009

This behavior seems to be intentional...
a) autofix is not available when there is a (nonempty) extends list (because it... can't really be fixed?)
b) the rule won't report when there's more than one property in the interface, regardless of the extends list.

The case you bring up falls where those two entirely separate behaviors interact

from typescript-eslint.

I'm surprised this has never come up before! Sometimes I just need to be able to access properties by index.

It's generally very, very rare that you want to add an unbounded indexer to an existing type!
It introduces a fair bit of unsafety (eg you can easily pass an invalid key).
Most people don't want to index an object with known keys using a raw string typed variable.

Or a safer way to do things would be eg

declare const key: string;
declare const base: AxiosError;
if (base.hasOwnProperty(key)) {
  base[key as keyof typeof base];
}

I don't think there's a really compelling, broadly-applicable reason to allow this within the rule.
I think leaving it as an error and having the user add a disable comment explaining why they're opting in to an indexer is a better outcome than just ignoring it and making it look like a false negative.

from typescript-eslint.

ok, so I've actually run into this issue yet again in a slightly different way. I have a large "base" interface and then I make several smaller and specific interfaces with Pick or Omit` for different use cases. I need them to be indexable.

Here's a playground link for everything below.

I could go this route:

interface ILargeBaseA {
  foo: string;
  bar: number;
  baz: boolean
  qux: string[];
  zop: null | string;
}

interface IStringsOnlyA extends Pick<ILargeBaseA, 'foo' | 'qux' | 'zop'> {
  [key: string]: string | string[] | null;
}

function getValA(obj: IStringsOnlyA, key: string) {
  return obj[key];
}

which works great and the return type of getValA is correctly inferred as string | string[] | null but this rule again complains that it would prefer it to be a Record.

Ok, I'll lean into that and move the indexing to the base class and use a Record as it wants

interface ILargeBaseB {
  [key: string]: string | string[] | boolean | number | null;
  foo: string;
  bar: number;
  baz: boolean
  qux: string[];
  zop: null | string;
}

type IStringsOnlyB = Pick<ILargeBaseB, 'foo' | 'qux' | 'zop'>;

function getValB(obj: IStringsOnlyB, key: string) {
  return obj[key];
}

ESLint no longer complains, but now I lose my index signature it it just infers an any type from that function. Using "strict":true this will give a compilation error

Element implicitly has an 'any' type because expression of type 'string' can't be used to index type 'IStringsOnlyB'.
  No index signature with a parameter of type 'string' was found on type 'IStringsOnlyB'.

as far as I can tell, my only workaround here is to disable this rule since it just seems flat out wrong my my use case here.

a possible workaround I might be able to do is use keyof like this, which accomplishes it all, but it may not always be feasible in certain situations

interface ILargeBaseC {
  [key: string]: string | string[] | boolean | number | null;
  foo: string;
  bar: number;
  baz: boolean
  qux: string[];
  zop: null | string;
}

type IStringsOnlyC = Pick<ILargeBaseC, 'foo' | 'qux' | 'zop'>;

function getValC(obj: IStringsOnlyC, key: keyof IStringsOnlyC) {
  return obj[key];
}

from typescript-eslint.

Why not something like

type IStringsOnlyC = Pick<ILargeBaseC, 'foo' | 'qux' | 'zop'> & Record<string, string | string[] | null>;

Or more generally

type IndexableThing = T & Record<string, values of T>;

What's wrong with an intersection here?

from typescript-eslint.

I do not use intersection types often, thanks for the reminder. ok, good call there.

but again, perhaps the rule could make more intelligent recommendations/fixes for cases like these?

from typescript-eslint.