mobx-state-tree icon indicating copy to clipboard operation
mobx-state-tree copied to clipboard

Typescript: Extracting type of value from a types.map(ModelType) in a generic function drops actions and views from returned type

Open evelant opened this issue 4 years ago • 7 comments

Bug report

  • [X ] I've checked documentation and searched for existing issues
  • [X ] I've made sure my project is based on the latest MST version
  • [X ] Fork this code sandbox or another minimal reproduction.

Sandbox link or minimal reproduction code https://codesandbox.io/s/withered-wind-ks771?file=/src/App.tsx

Describe the expected behavior Given the following function

export function filterMapValues<K, V>(m: Map<K, V>, predicate: (v: V) => any): V[] {
    const res: V[] = []
    for (const [k, v] of m) {
        if (!!predicate(v)) {
            res.push(v)
        }
    }
    return res
}

With the following input

const firstModel = t.model({}).views((self) => ({
  get foo(): string {
    return "foo";
  }
}));

const secondModel = t.model({
  myMap: t.map(firstModel)
});

const m = secondModel.create({ myMap: { test: {} } });

//Expect this to return [m]
const f = filterMapValues(m.myMap, (val) => val.foo === "foo");

Describe the observed behavior

Property 'foo' does not exist on type 'ExtractCSTWithSTN<IModelType<{}, { readonly foo: string; }, _NotCustomized, _NotCustomized>>'. Property 'foo' does not exist on type 'ModelCreationType<ExtractCFromProps<{}>>'.ts(2339

// Can't access the view "foo" on val
const f = filterMapValues(m.myMap, (val) => val.foo === "foo");

evelant avatar Aug 28 '21 01:08 evelant

Hey @evelant - thanks for putting together this issue. I'm sorry it's taken so long for us to get back to you.

TypeScript inference is a known problem with MST, and it's definitely something we're trying to improve. That said, I don't have a great fix for you right now off the top of my head.

If you found a solution to this, it would help a lot for others to see it. If you didn't and moved past it, no worries at all.

For now, I'm going to mark this as a TypeScript issue, label it that help is welcome, and hopefully we can find a path forward to make this kind of thing easier.

coolsoftwaretyler avatar Jun 28 '23 03:06 coolsoftwaretyler

More info/perhaps related - TypeScript seems to have trouble inferring things about nested models in types.map overall.

In this CodeSandbox, TypeScript complains about the todoStore types, even though they should be satisfactory.

But in this CodeSandbox, TS is happy with us for types.array

coolsoftwaretyler avatar Sep 29 '23 15:09 coolsoftwaretyler

An example a little closer to the original problem: https://codesandbox.io/s/clever-margulis-ks8fx7?file=/src/App.tsx. types.array is working here.

coolsoftwaretyler avatar Sep 29 '23 15:09 coolsoftwaretyler

Ooh, but array.filter method is typed with any, so maybe if we write a custom filter method this would break, too. I have to hop off for a while but I'd be curious if anyone else looks into that to see if this is just an issue with the generics overall.

coolsoftwaretyler avatar Sep 29 '23 15:09 coolsoftwaretyler

I will check tomorrow

chakrihacker avatar Sep 29 '23 17:09 chakrihacker

I have looked into the map implementation, there are few places with any tried fixing them, but still couldn't find the root where the types are getting changed

chakrihacker avatar Oct 03 '23 14:10 chakrihacker

Thanks @chakrihacker! We can keep this open and maybe revisit it over time. Lots of TypeScript work to be done, haha!

coolsoftwaretyler avatar Oct 06 '23 17:10 coolsoftwaretyler

So the major issue here is that IMSTMap<IT extends IAnyType> is almost a Map<string | number, IT["Type"]>, but not quite because of this one method:

set(key: string | number, value: ExtractCSTWithSTN<IT>): this

If we change value to IT["type"] you'll find TS infers the right thing for K and V. In its current form, I'm guessing V gets inferred as ExtractCSTWithSTN<IT> | IT["Type"] which is the same as ExtractCSTWithSTN<IT>.

That leads to the next problem: since this is a view, IT["CreationType"] obviously doesn't have that property (only the props!).

That sums up the problem. I'll think about if there's a solution or not, but I'm highly doubtful.

In the meantime, swap out Map<K, V> for Iterable<[K, V]> and all your problems go away! In general, I would highly recommend doing this. Not everyone is super into SOLID, but I've always found the "I" to be invaluable to great design, especially in a type system that's very structural. I like to think about it as "ask only for what you need".

thegedge avatar Feb 25 '24 01:02 thegedge

Thanks @thegedge - this is an excellent solution. Really appreciate your insight.

I'm going to convert this issue to a discussion, which will close it. I forked the original reproduction and made the Iterable change, and thing are working over here: https://codesandbox.io/p/sandbox/inspiring-neco-5t5dsq

Here's the code for posterity:

import "./styles.css";
import { types as t } from "mobx-state-tree";

/**
 * Like array.filter except on a map, returns value[] array of found results
 * @param m
 * @param predicate
 * @returns
 */
export function filterMapValues<K, V>(
  m: Iterable<[K, V]>,
  predicate: (v: V) => any
): V[] {
  const res: V[] = [];
  for (const [k, v] of m) {
    if (!!predicate(v)) {
      res.push(v);
    }
  }
  return res;
}

const firstModel = t.model({}).views((self) => ({
  get foo(): string {
    return "foo";
  },
}));

const secondModel = t.model({
  myMap: t.map(firstModel),
});

const m = secondModel.create({ myMap: { test: {} } });

//Views don't exist when extracting the value type of
//an instance from a map in a generic function.
//Instead we seem to get only the props.
const f = filterMapValues(m.myMap, (val) => val.foo === "foo");

export default function App() {
  return <div className="App"></div>;
}

coolsoftwaretyler avatar Feb 26 '24 01:02 coolsoftwaretyler