Generate types

[chrisdrackett]

This is a quick proof of concept based on this comment: #45 (comment)

I've created a 6th example that just takes the first example and adds a User to create a circular dependency. I then created types for both Todo and User:

https://github.com/mobxjs/mst-gql/blob/feature/typeGeneration/examples/6-typegen/src/app/models/TodoModel.ts#L18

https://github.com/mobxjs/mst-gql/blob/feature/typeGeneration/examples/6-typegen/src/app/models/UserModel.ts#L16

These seem to work as expected when used (https://github.com/mobxjs/mst-gql/blob/feature/typeGeneration/examples/6-typegen/src/app/components/Todo.tsx#L7, https://github.com/mobxjs/mst-gql/blob/feature/typeGeneration/examples/6-typegen/src/app/Home.tsx#L19)

Things I'm not (yet) sure how to do:

• have these types be used by default so it isn't necessary to import and cast
• generate the base types based on the graphQL schema (this seems straight forward)
• generate or infer (?) user-added actions/props/etc. to the above generated type (toggle on Todo in this example)
• I'm guessing there are things my model is missing from the types generally added my MST.

[zenflow]

The issue here is a bit over my head since I'm not a real TS user, so I can't give much review.

My only concern is the growing number of example projects we need to maintain & keep up-to-date. Is there a way we can combine this into one of the existing examples? (And possibly go even further and consolidate examples into two or theee that demonstrate all features/behaviors (which I would be happy to work on)?

[chrisdrackett]

I would say this does not need to stay an example outside this PR. I just added it as a playground so we can discuss and work on this feature. Once we have it working I think we can trash the example pre-merge

[godness84]

@chrisdrackett at https://github.com/godness84/mst-gql/tree/fix-circular-refs you can find my attempt to solve the circular refs issue. Instead of defining exhaustively each type, I leverage the Instance<typeof ...> type: in this way I just need to explicit the fields that refer other types.

My attempt consists of:

• defining a base model without any reference to other models. (BaseModelWithoutRefs).
• defining another base model (extending BaseModelWithoutRefs) with references but casting it to BaseModelWithoutRefs. (in this way mobx-state-tree knows about references but not typescript)
• defining the type of the base model just with the references. (it will be used in the next step, let's call it BaseModelRefsType)
• defining the actual model (extending the base model)
• exporting the type of the actual model merging Instance<typeof MyModel> with BaseModelRefsType.

Moreover, I cast the self argument to the type of the model with a nasty trick. (createSelfWrapper)

Results:

• it compiles 🎉
• typescript intellisense works even on deep and circular references 🎊

The bad:

• in .tsx files, typescript intellisense does not warn about fields that do not exists. It will complain only when I compile, not using VSCode.
• my code is in spaghetti style and should be refactored.

I really appreciate your feedback and the feedback given by the people that will venture into my fork!

[beepsoft]

Wow, I'm really excited whether this could solve #45.

I did a quick trial and it mostly seems OK, although I still get 'any' for some of my complex models.

One thing I noticed is that you generate two interface definitions with the same name for each model here:

  format === "ts"
    ? `/* The TypeScript type of an instance of ${name}ModelBase */
export interface ${name}ModelType extends Instance<typeof ${name}Model.Type> {}
export interface ${name}ModelType extends ${name}ModelBaseRefsType {}

Is this correct?

[godness84]

@beepsoft yes that's correct! https://www.typescriptlang.org/docs/handbook/declaration-merging.html

In which cases do you get any? Would you mind to post here the graphql schema?

[chrisdrackett]

@godness84 I made another PR based on your branch just so I could see the changes. I didn't get done playing with it, but it was looking good so far! I'll spend more time tomorrow :)

[beepsoft]

In which cases do you get any? Would you mind to post here the graphql schema?

@godness84 I have this Hasura generated schema:

https://gist.github.com/beepsoft/84a64102a80a3ac7eb1f656b64fc2af8

It is quite complicated, but the main entities are choices and polls:

# columns and relationships of "choices"
type choices {
  id: ID!

  # An object relationship
  poll: polls!
  poll_id: bigint!
  text: String
}

# columns and relationships of "polls"
type polls {
  # An array relationship
  choices: [choices!]!

  created_at: timestamp
  created_by: bigint
  expiration_date_time: timestamp!
  id: ID!
  question: String
  updated_at: timestamp
  updated_by: bigint
}

I generated the model like so:

node generator/mst-gql-scaffold.js --format ts --roots polls,choices --outDir model-poll-godness84 schema.graphql

And I get errors like:

Error:(13, 14) TS7022: 'choicesModel' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.

[godness84]

@beepsoft I tried your schema at https://gist.github.com/beepsoft/84a64102a80a3ac7eb1f656b64fc2af8 and I don't see any error.

Few questions:

• In which file do you see that error?
• Which version of typescript are you using?
• Did you clean the model-poll-godness84 folder before launching the generator? If you don't use the --force option, model files will not be overwritten, only base model files.

[beepsoft]

• I saw the error in choicesModel.ts. I use IntelliJ and I moved my cursor over choicesModel in line export const choicesModel = choicesModelBase and it displayed the error above. I also saw the error I wrote previously in the Typescript Error console of IntelliJ.
• I tried TS 3.4.5 and TS 3.6.4
• Yes, I did a clean generation, removed the directory before generating the models

If it works for you, It could very well be some problem with my environment. Could there be any tsconfig.json value that may effect the compilation?

[godness84]

If I set "noImplicitAny": true in tsconfig.json I also see the error. With "noImplicitAny": false there is no error (of course), and if I place the cursor over choicesModel I see that it's considered any. Anyway, inside the sample action log if you start typing self. you should see the correct suggestions.

[beepsoft]

Yep, I can see that self. autocomplete works correctly!

Anyway, I think your solution will work, hopefully @chrisdrackett can merge and publish it soon.

[godness84]

@beepsoft the any issue is caused by that nasty trick to automatically cast self argument (using the as function created with createSelfWrapper). If you strip the as function out from the actions definition, typescript will stop complaining about the model being any.

Another solution could be to use an as function to cast the self argument only when you need to access self. Just as an example:

function as(self: any) { return self as unknown as MyModelType }

const MyModelType = BaseModelType
  .actions(self => ({
    doSomething() {
      const childName = as(self).child.name // we account there's a child reference with name field.
      // do whatever you want with your child name
    }
  })

[chrisdrackett]

@godness84 I merged your changes with my example and I'm still running into issues around:

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

can you take a look and make sure I didn't miss anything?

#128

[godness84]

@chrisdrackett The issue comes from the usage of createSelfWrapper (that trick to cast self argument to the model instance type). As anticipated here (#110 (comment)) I fixed it wrapping only the self argument instead of the entire function describing actions.

Moreover, I added better typing support to arrays of referenced models using a IObservableArray<ModelType> type instead of a simple ModelType[]

I updated my branch (https://github.com/godness84/mst-gql/tree/fix-circular-refs), should I push it somewhere else?

[chrisdrackett]

no, I can pull it into #128 and do some more testing!

[chrisdrackett]

@godness84 moving discussion on this issue to #128 so its more directly linked to the code being discussed!