Real-World Problems & Thoughts

[RXminuS]

There are a few things we've noticed working with mstQL that I think we might need to address. Please share your own experiences if you're using this in "production" as well.

1. Unwieldy Codebase

The number of models can quickly grow to unmanageable size for a simple API. Ideally, we'd be able to leave most models alone and only implement one if we actually have something to add to it. One solution could be to by default export the FoodModel class that extends FooBaseModel from the actual FooModel.base file unless you specifically add a FooModel file alongside it?

Then we could simply generate a models.base.ts file that looks for Models that extend its functionality and imports them?

2. Interoperability With "normal" MST

Not every Model is represented in the API. For instance, we have a conceptual model for the Editor that contains some state about which filters are active etc. It references the current document which does come from the API.

I think we need a good base class (not MSTGQLBaseModel) that can be mixed into any normal MST model that adds the store() method and a few other conveniences. Or at least a few good examples on how API models and non-api models work together. Also, at the moment it becomes really difficult to see which models come from the API, especially since we have about 60 of them.

Instead of generating the index.ts/js file instead I think we should generate an graphql.internal.ts file (in line with #117) as well as an graphql.external.ts/js file that you can export from your own index.ts. That way you can add additional modules to that index.ts (or your own internal.ts for that matter) and simply import like

import { NonGraphQLModel } from "."

3. Circular dependencies EVERYWHERE

It's just the nature of GraphQL. You re-use the models and their field export types that it in turn depends on. We find our code constantly breaking simply by adding a

FooModel = FooModelBase.named("Foo").actions(self => ({
    something: flow(function*(){
       const res = yield self.store().mutateSomething();
    })
}))

If the return value of the mutate has a field that is of type FooModel since now the generator depends on that type too. I know we already have an open issue for moving away from inferred Typescript Types and I think it's crucial we do so ASAP!

[RXminuS]

One idea that I had is to draw some inspiration from how GraphQL Code Generator creates resolvers.

You would start to specify in the configuration which models to import for composition:

extend:
    Foo:
        - some-module#FooExtension

Where the extension would look like

some-module/index.ts

import { FooModelBaseType } from "../mstql.types.ts"

export const FooExtension = BaseModel<FooModelBaseType>.named("FooExtension").props({
    happy: true
}).views(self => ({
   get veryHappy(){
      return self.apiData.length > 0 && !self.happy;
   }
});

Then we would simply generate a single mstql.ts file that imports the models that you want and does something like:

mstql.types.ts

export interface RootStoreBaseType {
   
}

export interface FooModelBaseType {
   apiData: string,
   store(): RootStoreType
}

export interface RootStoreType<T> {
   //TODO: Not sure how to do this yet, we might have to omit some of the base properties or something
  // or take an entirely different route.
}

export interface FooModelType<T>{

}

mstql.ts

import { FooModelExtension } from "some-module";
import { FooModelType } from "mstql.types";

export const FooModel : FooModelType<typeof FooExtension> = compose(
    MSTGQLBaseModel.named("Foo").props({}),
    FooExtension
);

[RXminuS]

Also another thought.

I think we should potentially only generate Typescript code, because we can always pass it through a type omission process, much easier than we can add types after the fact.

And having to decide in the generators whether or not to generate something quickly becomes messy.

[chrisdrackett]

I'm a little confused what problem is trying to be solved by the above. Is the concern there are too many files in the models folder? Or is it something about how the files are currently generated?

I like the idea of generating typescript and they (if the user wants js) just stripping the types out. This sounds like something that could be its own ticket.

[RXminuS]

Yeah so partially the problem is that there are a lot of models generated, and only a very small subset actually end up having our own code. I'd much rather have a single mstql.ts file that holds all the boilerplate and then imports the relevant extensions to those models compared to about 80+ boilerplate.ts files that are mostly untouched.

But it would also make it a lot easier to generate new versions because we don't have to look if there's a ModelName.ts file that pre-exists.

The other problem that you run into with mstQL, especially in larger models is that because in GraphQL you constantly re-use models in return types that it's really really easy to end up with circular references and imports. This automatically breaks the type inference (thus we need static types) but also import order becomes really important. Putting everything in a single file allows you to simply make use of variable hoisting and avoid complex "internal.ts" etc import schemes.

Unfortunately, I can't share the codebase where we are having these problems yet so it's a bit hard to illustrate.

[mweststrate]

N.B. note that it is configurable for which types models get generated The single file idea sounds very interesting though, something worth exploring.

…

On Thu, Oct 3, 2019 at 7:15 PM Rik ***@***.***> wrote: Yeah so partially the problem is that there are a lot of models generated, and only a very small subset actually end up having our own code. I'd much rather have a single mstql.ts file that holds all the boilerplate and then imports the relevant extensions to those models compared to about 80+ boilerplate.ts files that are mostly untouched. But it would also make it a lot easier to generate new versions because we don't have to look if there's a ModelName.ts file that pre-exsists. The other problem that you run into with mstQL, especially in larger models is that because in GraphQL you constantly re-use models in return types that it's really really easy to end up with circular references and imports. This automatically breaks the type inference (thus we need static types) but also import order becomes really important. Putting everything in a single file allows you to simply make use of variable hoisting and avoid complex "internal.ts" etc import schemes. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#119?email_source=notifications&email_token=AAN4NBHNCXYNYMS23ACGRKTQMYZDNA5CNFSM4I4ZFPBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAJDDCI#issuecomment-538063241>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAN4NBAVKCN6TT33LKRDQSLQMYZDNANCNFSM4I4ZFPBA> .

[RXminuS]

Ah I had missed that!

I'll mock it out the single-file idea in the Gluegun version I'm working on then we can have a more productive discussion :-)

[mweststrate]

Perfect, looking forward!

…

On Thu, Oct 3, 2019 at 7:49 PM Rik ***@***.***> wrote: Yeah, the problem is that we actually want models generated for all the API models, but that all of it is basically boilerplate. We only add our own functionality to 5-10 of those models but I still have to dig through about 80 files to find them. I'll mock it out the single-file idea in the Gluegun version I'm working on then we can have a more productive discussion :-) — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#119?email_source=notifications&email_token=AAN4NBCI7OYCV2T42DWDNU3QMY5E5A5CNFSM4I4ZFPBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAJGLZI#issuecomment-538076645>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAN4NBATLFUKNHLJA2YZGQDQMY5E5ANCNFSM4I4ZFPBA> .

[chrisdrackett]

I'm going to close this issue for now mostly because I don't feel like its actionable. Please feel free to continue this discussion on spectrum and/or open issues mentioned above that are actionable!

[Matth10]

@RXminuS What the status of the spike with Gluegun ? We are currently doing some refactoring and are really interesting with this lib. However we have about 100 models and this result with a lot of circular dependencies (134). I think this one file idea could resolve our problem.