TypeScript performance

[RWOverdijk]

Question

• I've checked documentation and searched for existing issues
• I tried the spectrum channel

I have no idea where or why exactly but sometimes my IDE (webstorm) goes insane, running 100% on all my cores in some odd cases.

I've found that if I do not specifically state the source of a store for example, this happens. One such example is:

export class SpaceScreen extends Component<SpaceProps, SpaceState> {
  get venueStore() {
    return this.props.rootStore.venueStore as Instance<typeof VenueStore>;
  }
}

If I don't add the as Instance<typeof VenueStore> webstorm tries to destroy my macbook.

Another issue I've found is using getParent (rootStore) never allows me to type properly, saying:

... implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer

if I try:

const rootStore = (getParent(self) as Instance<typeof RootStore>);

Maybe this is something that's known, but it's pretty hard to figure out why these things happen.

PS: Are there plans to give typescript more love with MST? Or doesn't it need any and is my thought on that wrong?

[Bnaya]

it happens to me in vs code as well.
it's due to the inference complexity.

Take a look on:
microsoft/TypeScript#25023 (comment)
in the linked comment there's my "workaround"

[RWOverdijk]

I'll give that a go. But it doesn't fix the:

implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer

Do you have any tricks for that as well? (besides typing rootstore as any explicitly).

Also: is it an idea to think about an overhaul? Maybe something like classy-mst but with decorators for example? I love the way MST is supposed to work, but truth is that it can be a headache with typescript (especially for example when there's a TS error where the message is the size of a mountain). I'd love to work on that as well.

[Bnaya]

Take a look at:
https://codesandbox.io/s/mst-issue-1406-olsn3

[RWOverdijk]

@Bnaya I meant this: https://codesandbox.io/s/brave-lederberg-1dqpo?fontsize=14

Look in TestStore.ts.

[terrysahaidak]

@RWOverdijk did you try to use generic param with getParent?

[kenaku]

implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer

This happens when you use getRoot() or getParent() in views/actions.
But you have to make it in ALL of your views/actions across your store to fix issue.
You have to explicitly set return values of that views/actions (including flow actions)
Also, when we refactored whole store like this, CPU usage by Typescript dropped significantly.

Example:

    get store(): IStore {
      return getRoot(self)
    },
    get language(): string {
      return this.store.language
    },

[RWOverdijk]

@kenaku I don't understand what you're saying. Having a getter (assuming you mean a view?) like this won't work if I use "this" and the context isn't the same (which happens frequently).

Also not sure what "IStore" is.

In any case I tried it and it doesn't solve the issue (you can solving try it here: https://codesandbox.io/s/brave-lederberg-1dqpo?fontsize=14 and see what I mean).

[RWOverdijk]

@terrysahaidak It's RWOverdijk 😄 And it doesn't make a difference. I tried getParentOfType, I tried getParent with generic typeof RootStore and I tried generic Instance<typeof RootStore>. All of which yield the same result. Also reproducable in the codesandbox link.

[mweststrate]

The error you get is correct, as TypeScript doesn't support recursive types at the moment (3.7 might change that!). Since the type of your store depends on the output on the signature of that view, which, to determine it's result type, needs to know the type of.... you see the circle quickly.

However, since interfaces can be recursive, you can fix this by using the trick explained in the docs: Make sure you introduce the type export interface I{MODEL} extends Instance<typeof {MODEL}> {} for every model type you create.

After that, you can use that interface wherever needed to 'break' the cycle in the types. Without loosing strong typing.

I applied the change here: https://codesandbox.io/s/serene-fog-oryt3. Note that calling .test is now strongly typed again. With this trick and sprinkling these interface any cycles can be fixed afaik. it might not always be immediately clear where you need it, but if you strongly type all action / view returns, it gets you quite far. (And I think that this a really good practice anyway).

It is a bit cumbersome, but usually straightforward mechanism. So I guess we have to live with it until TS supports recursive types 😅

An added benefit is that it makes TS a lot faster as well.

[terrysahaidak]

Sorry, @RWOverdijk, I've been typing from my phone without autocomplete :(

@mweststrate any link where I can read about that TS update? Don't remember anything related to cyclic types.

[RWOverdijk]

@mweststrate Well well well... Look at that. I see it requires a two-step approach of having rootStore first and then getting a nested store from that. I suppose I can live with that.

Isn't this whole "ISomeInterface" thing against the best practises/style guide? https://basarat.gitbooks.io/typescript/content/docs/styleguide/styleguide.html#interface

I don't know how much of an authority that doc is. I just remember a c# person saying they regret that convention and thought it would only make sense to then not apply it to TS.

Also, now that I have your attention... Are there plans to do a rewrite/happier implementation written using typescript? I have some ideas on how I'd like that to work and was thinking of folding my own solution. You're working on immer (which btw does make me wonder, is that because you no longer like mst? Or because you have bigger plans? Or do you consider them unrelated?)) and I don't want to put in effort somewhere if you end up working on exactly what I'm looking to build.

I know, this issue is not an AMA but you know. At least I tried 😄

[mweststrate]

Yes, I hate the I as well. However, since you need to find a name that doesn't collide with your model type (although, maybe colliding is actually fine, since one is a value and the other a type)..... So yeah, feel free to leave out the I if that works :) For an MST alternative that is better at TS, make sure to check https://github.com/xaviergonz/mobx-keystone Immer is solving an quite unrelated problem, and MST I never worked that much on myself :)

…

On Mon, Oct 28, 2019 at 7:16 PM Roberto Wesley Overdijk < ***@***.***> wrote: @mweststrate <https://github.com/mweststrate> Well well well... Look at that. All right, I can live with that. Isn't this whole "ISomeInterface" thing against the best practises/style guide? https://basarat.gitbooks.io/typescript/content/docs/styleguide/styleguide.html#interface I don't know how much of an authority that doc is. I just remember a c# person saying they regret that convention and thought it would only make sense to then not apply it to TS. Also, now that I have your attention... Are there plans to do a rewrite/happier implementation written using typescript? I have some ideas on how I'd like that to work and was thinking of folding my own solution. You're working on immer *(which btw does make me wonder, is that because you no longer like mst? Or because you have bigger plans? Or do you consider them unrelated?))* and I don't want to put in effort somewhere if you end up working on exactly what I'm looking to build. I know, this issue is not an AMA but you know. At least I tried 😄 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1406?email_source=notifications&email_token=AAN4NBAAPPUD2ZOJKOGHVB3QQ43ANA5CNFSM4JFF4ZF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECOCJOI#issuecomment-547103929>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBHPATZNXTIKG7RNYTTQQ43ANANCNFSM4JFF4ZFQ> .

[RWOverdijk]

@mweststrate Nice, thanks for sharing that. I'll take a look.

I'll name my interfaces something other than II118nStore for now 😄 Maybe just a clear name like I18nStoreInstance and I18nStoreSnapshotIn. A bit long but clear.

Thanks again, everything works now :)

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.