Circular dependencies between models #45

chrisdrackett · 2019-06-29T23:59:10Z

I'm running into the following error on all my models that are related to each other:

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

Here is the file in question:

UserModel.base.ts

...
export const UserModelBase = MSTGQLObject
  .named('User')
  .props({
    __typename: types.optional(types.literal("User"), "User"),
    createdAt: types.maybe(types.frozen()),
    email: types.maybe(types.string),
    id: types.identifier,
    name: types.maybe(types.string),
    projects: types.optional(types.array(MSTGQLRef(types.late((): IAnyModelType => ProjectModel))), []),
    updatedAt: types.maybe(types.frozen()),
  })
...

Project has a link back to the User model. When I first ran into this I found:

https://github.com/mobxjs/mobx-state-tree#handle-circular-dependencies-between-files-and-types-using-late

and thought I could get around this by editing UserModel.ts like the following:

export const UserModel = UserModelBase.props({
  projects: types.optional(
    types.array(MSTGQLRef(types.late((): IAnyModelType => ProjectModel))),
    [],
  ),
})

But I still get the error in typescript. It seems like I need to edit UserModel.base.ts directly to fix this issue, but that is a generated file.

The text was updated successfully, but these errors were encountered:

Amareis · 2019-07-05T08:41:34Z

+1, in my (complicated enough) domain models generated models are useless because of circular dependencies.

Amareis · 2019-07-05T08:48:50Z

Against the comment in https://github.com/mobxjs/mst-gql/blob/master/generator/generate.js#L354, types.late isn't enough to prevent circular dependencies :)

elie222 · 2019-07-05T08:52:01Z

@chrisdrackett does editing the generated file solve your problems? (I realise this isn't a long term fix).

@Amareis is the problem with TS types or a JS issue?

Amareis · 2019-07-05T08:59:07Z

@elie222 just TS typings. For correct circular references handling we need to use types.late((): IAnyModelType => SomeModel), but if we do it everywhere, all references will be any type. So, this should be applied only at some points, to break the wheel, but currently IDK how to select that points algoritmically.

Amareis · 2019-07-05T09:44:33Z

https://github.com/pahen/madge can be helpful in find and resoving circular deps. My current models have 36 of it.

chrisdrackett · 2019-07-05T21:22:58Z

@elie222 editing the generated file does fix the problem, but like @Amareis mentioned because this makes the return type any you only want to do it where needed and not on everything. I'm not sure if that is something the library could detect, but if so, it would be awesome.

I can try and add a note about this if needed to the docs. Its really more a MST thing than a mst-gql thing, but might be worth letting users know about regardless.

elie222 · 2019-07-11T22:03:51Z

So I'm thinking we can handle this by manually generating the TS types. TS shouldn't have issues with the circular dependencies in general. For example, the following code compiles fine:

interface Post {
  id: string
  comment?: Comment
}

interface Comment {
  id: string
  post: Post
}

const post: Post = {
  id: 'a',
  comment: {
    id: 'b',
    post: {
      id: 'c',
    }
  }
}

Having TS figure out the types from the model itself is nice usually, but as we're generating this code in any case, we may as well generate the TS types too and get around this circular any issue.

@mweststrate I know you've dealt with circular dependencies quite a lot. What are your thoughts on this?

Amareis · 2019-07-12T10:30:38Z

Yep, that's will really cool! Besides of circular deps handling, it'll make nice typenames in compiler errors, instead of usual mst types garbage.

mweststrate · 2019-07-12T10:33:01Z

I think it is a really cool idea! Be aware to use interfaces instead of types wherever possible, as types in TS can't have circular refs either

…

On Fri, Jul 12, 2019 at 12:30 PM Иван Плесских ***@***.***> wrote: Yep, that's will really cool! Besides of circular deps handling, it'll make nice typenames in compiler errors, instead of usual mst types garbage. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#45?email_source=notifications&email_token=AAN4NBGSB3PNVX6PFMYTPELP7BMM5A5CNFSM4H4LOAR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZZMGMQ#issuecomment-510837554>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAN4NBELEH6JNRCBMW6UNCLP7BMM5ANCNFSM4H4LOARQ> .

mweststrate · 2019-07-17T20:53:21Z

Regardless on whether the above is implemented or not, in general two tips that help with breaking circular type dependencies:

Break circles by not relying on type inference for method arguments / return types (example: Circular type references with TS 3.5 mobx-state-tree#1341 (comment))
Use interfaces that extends types (example: https://github.com/mobxjs/mobx-state-tree#using-a-mst-type-at-design-time, the second code snippet)

Hope that helps in the mean time!

chrisdrackett · 2019-07-26T03:36:32Z

@elie222 have you done any work on this? I was thinking of giving it a try tomorrow and/or over the weekend

elie222 · 2019-07-26T07:18:57Z

I have not unfortunately :( too busy right now

…

On Fri, 26 Jul 2019 at 6:36 Chris Drackett ***@***.***> wrote: @elie222 <https://github.com/elie222> have you done any work on this? I was thinking of giving it a try tomorrow and/or over the weekend — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#45?email_source=notifications&email_token=AAXSQXY4A6D2LK2B2346BZ3QBJWMDA5CNFSM4H4LOAR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD23MU3I#issuecomment-515295853>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXSQX2KVTMW7PNKDNVOBYTQBJWMDANCNFSM4H4LOARQ> .

mweststrate · 2019-08-02T20:54:13Z

#69 should already make this much better

beepsoft · 2019-09-04T13:25:38Z

So I'm thinking we can handle this by manually generating the TS types.

Wouldn't it be possible to use graphql-codegen (https://graphql-code-generator.com) for this? It can generate TS interfaces for the same graphql schema we use in mst-gql-scaffold.js.

elie222 · 2019-09-04T13:59:12Z

The MST types are different to what graphql codegen will produce. But there are places this project could have used graphql codegen. It was lightly discussed in some another issue.

…

On Wed, 4 Sep 2019 at 16:25 beep ***@***.***> wrote: So I'm thinking we can handle this by manually generating the TS types. Wouldn't it be possible to use https://graphql-code-generator.com for this? It can generate TS interfaces for the same graphql schema we use in mst-gql-scaffold.js. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#45?email_source=notifications&email_token=AAXSQX7CFYPQGIYMEUQKKP3QH6ZNFA5CNFSM4H4LOAR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD53RW6Q#issuecomment-527899514>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXSQX3PIHIU3EBEQ2KD3MDQH6ZNFANCNFSM4H4LOARQ> .

beepsoft · 2019-09-04T14:07:38Z

@elie222 I see. And how should this MST compatible TS interface look like? Do you have any idea of this?

elie222 · 2019-09-04T14:22:38Z

Take a look here: https://github.com/mobxjs/mobx-state-tree#typescript-and-mst If you're running MST in your project you should be able to hover over items and see their types. You can do something like this to get TS types from an MST model in a regular MST project: type ISearchStoreType = typeof SearchStore.Type export interface ISearchStore extends ISearchStoreType {}

…

On Wed, 4 Sep 2019 at 17:07, beep ***@***.***> wrote: @elie222 <https://github.com/elie222> I see. And how should this MST compatible TS interface look like? Do you have any idea of this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#45?email_source=notifications&email_token=AAXSQXZGK24BLLKNEIM5EDDQH66KXA5CNFSM4H4LOAR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD53V7TA#issuecomment-527917004>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXSQX4JJEO7WWIVX2BRA73QH66KXANCNFSM4H4LOARQ> .

beepsoft · 2019-09-04T15:07:55Z

Thanks for these details!

Another idea/question: implementing this in the generator as a general mechanism for any late() type:

mobxjs/mobx-state-tree#943 (comment)

We would then always have the actual prop as of type any (because of IAnyModelType) and a matching view, which returns the prop casted using Instance<...> and would use this view function in code where we want hints from the IDE.

This is still hackish but could be an optional workaround until manually generated TS types happen.

Implements the trick mentioned in this comment: mobxjs/mobx-state-tree#943 (comment) Every late() reference field will be of type IAnyModelType and will be accompanied by a view getter casting it to the actual type of the prop. After this a reference field name "refField" should be accessed as "refFieldProp" which will provide correct typings/autocomplete in IDEs. This trick works up to typescript 3.4.x and doesn't work in 3.5 and above :-( Using Instance<...> like this however still works in all versions: mobxjs/mobx-state-tree#943 (comment) refs mobxjs#45

beepsoft · 2019-09-05T07:39:24Z

I implemented this solution above. It works nicely with TS 3.4, but doesn't work with newer versions.

chrisdrackett · 2019-09-05T15:29:43Z

there seems to be something going on with TS versions past 3.4 that make things like this project and others I use (graphql-nexus/nexus-prisma#291) have issues. I haven't taken the time to dig into it (for now I'm just locked to 3.4.5).

beepsoft · 2019-09-24T12:15:09Z

Is there anyone working on @elie222's proposal about generating TS types "manually"?

So I'm thinking we can handle this by manually generating the TS types. TS shouldn't have issues with the circular dependencies in general. For example, the following code compiles fine:
interface Post {
  id: string
  comment?: Comment
}

interface Comment {
  id: string
  post: Post
}

const post: Post = {
  id: 'a',
  comment: {
    id: 'b',
    post: {
      id: 'c',
    }
  }
}
Having TS figure out the types from the model itself is nice usually, but as we're generating this code in any case, we may as well generate the TS types too and get around this circular any issue.

@mweststrate I know you've dealt with circular dependencies quite a lot. What are your thoughts on this?

elie222 · 2019-09-24T13:09:44Z

I haven't touched it all unfortunately.

…

On Tue, 24 Sep 2019 at 15:24, beep ***@***.***> wrote: Is there anyone working on @elie222 <https://github.com/elie222>'s proposal about generating TS types "manually"? So I'm thinking we can handle this by manually generating the TS types. TS shouldn't have issues with the circular dependencies in general. For example, the following code compiles fine: interface Post { id: string comment?: Comment } interface Comment { id: string post: Post } const post: Post = { id: 'a', comment: { id: 'b', post: { id: 'c', } } } Having TS figure out the types from the model itself is nice usually, but as we're generating this code in any case, we may as well generate the TS types too and get around this circular any issue. @mweststrate <https://github.com/mweststrate> I know you've dealt with circular dependencies quite a lot. What are your thoughts on this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#45?email_source=notifications&email_token=AAXSQX2DFIHY6D2ZOCPDOKDQLIBH5A5CNFSM4H4LOAR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7OET3A#issuecomment-534530540>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXSQX4RVIN5LAVR4JKBUNTQLIBH5ANCNFSM4H4LOARQ> .

beepsoft · 2019-09-24T13:35:23Z

I may try to implement it, but I don't know what actually needs to be done here.

Can you give me some hints how it would work? How these generated TS types would be associated with the MSTGQLObject based MST models the generator generates? If that's how it is planned to work at all - I'm a little bit lost here.

chrisdrackett · 2019-09-24T16:12:58Z

I think after the types are generated we would just associate them with the return of RootStore.create, for example here: https://github.com/mobxjs/mst-gql/blob/master/examples/1-getting-started/src/app/index.tsx#L11

I think also ideally the user should be able to easily extend these types in case they are adding props, views, actions, etc. to a store themselves.

chrisdrackett · 2019-09-24T17:22:00Z

@beepsoft would it be helpful for me to attempt to put together example output for some of the examples in this repo? That might give us a target to discuss before we worry about how to get from the input to output.

beepsoft · 2019-09-25T04:53:58Z

@chrisdrackett Yes, an actual example would be a great starting point, thank you!

RXminuS · 2019-09-25T13:25:49Z

I'm a bit swamped to have time to help out so instead I left a $50 bounty on bountysource :-)

chrisdrackett · 2019-09-25T17:09:33Z

@beepsoft ok, I'll work on it some today. Not sure if I'll get anything to share as I probably need to learn a lot more about how MST types work in general :D

chrisdrackett · 2019-09-25T18:09:35Z

I went ahead and put together a draft branch for this work here: #110

This was a quick first pass and certainly missing a lot. It also currently does not do any generation.

Hopefully this can act as a discussion starter!

RXminuS · 2019-09-30T11:52:29Z

#112 is probably somewhat related

beepsoft · 2019-09-30T13:44:28Z

@RXminuS does #112 solve the circular dependencies problem? Or which part of it?

kpogodin · 2019-10-01T09:52:41Z

+1 i have circular deps in gql

base file generate at base model field

import { UserGroupModel } from "./UserGroupModel"

export const UserGroupModelBase = MSTGQLObject
  .named('UserGroup')
  .props({
 //
    manageableUserGroups: types.optional(types.array(MSTGQLRef(types.late(() => UserGroupModel))), []),
//
  })`

at the same time custom model required import UserGroupModelBase

import { UserGroupModelBase } from './UserGroupModel.base';

export const UserGroupModel = UserGroupModelBase.actions(self => {
  return {
    addRoleId(id) {
///

as a result -> circular deps + build error
Cannot read property 'actions' of undefined at UserGroupModel

pvpshoot · 2019-10-01T10:55:38Z

and this is QL schema

RXminuS · 2019-10-01T21:42:11Z

@beepsoft I've started a new PR that's a bit better #117 . The issue that it solves is that in certain cases the actual import is not yet defined depending on the loading order. By using the internal.js module you get more control over the exact loading order. However, it doesn't solve anything about the circular type declarations preventing proper type inference on models that reference themselves.

RXminuS · 2019-10-01T21:43:20Z

@gadzillllla Can you try with #117 and reorder the internal file to see if you can make the module load sooner?

pvpshoot · 2019-10-02T07:50:24Z

@RXminuS we tried it, and it works fine for now

beepsoft · 2019-10-10T19:39:48Z

I just tried if it is any better with TS 3.7.0-beta, but unfortunately it is not.

onomated · 2019-10-28T10:28:23Z

@RXminuS had the same issue as @gadzillllla, JS codebase with circular dependencies. Tried running your fork and got this error while generating the gql models

  - [OBJECT] __InputValue
  - [OBJECT] __EnumValue
  - [OBJECT] __Directive
  - [ENUM] __DirectiveLocation
/app/node_modules/mst-gql/generator/mst-gql-scaffold.js:115
    moduleLoadingOrder
    ^

ReferenceError: moduleLoadingOrder is not defined
    at main (/app/node_modules/mst-gql/generator/mst-gql-scaffold.js:115:5)
    at Object.<anonymous> (/app/node_modules/mst-gql/generator/mst-gql-scaffold.js:121:1)
    at Module._compile (internal/modules/cjs/loader.js:936:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:947:10)
    at Module.load (internal/modules/cjs/loader.js:790:32)
    at Function.Module._load (internal/modules/cjs/loader.js:703:12)
    at Function.Module.runMain (internal/modules/cjs/loader.js:999:10)
    at internal/main/run_main_module.js:17:11
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

mst-gql.config.js

module.exports = {
  force: true,
  format: "js",
  input: "app/graphql/schema.graphql",
  outDir: "app/frontend/models",
  roots: [] // auto-detect
};

beepsoft · 2019-10-28T11:22:59Z

@RXminuS this PR seems to provide a solution:

#128

Will be hopefully merged into master soon.

onomated · 2019-10-28T12:54:47Z

#128 is generating TS for JS spec:

export const RootStoreBase: typeof RootStoreBaseNoRefs = RootStoreBaseNoRefs

Also exhibiting the same webpack circular dependencies error while loading app:

fromRequireContextWithGlobalFallback.js:21 Error: Cannot find module 'mst-gql'
    at webpackMissingModule (reactUtils.js:1)
    at Module../app/frontend/models/reactUtils.js (reactUtils.js:1)
    at __webpack_require__ (bootstrap:19)

beepsoft · 2019-10-28T13:00:02Z

(@onomated wrong @ in previous message, sorry :-) )

I've never seen such webpack error, sorry, then it is a different issue.

chrisdrackett · 2019-11-22T01:05:37Z

fixed in 0.7.0

beepsoft · 2019-11-22T10:02:54Z

🎉 Great work @godness84, thanks for your solution and @chrisdrackett for managing the merge! I'm so happy with this issue being solved!

godness84 · 2019-11-22T10:30:54Z

Thanks to you guys!

michaelspeed · 2020-07-28T16:44:09Z

still facing issue with circular deps

Circular dependency detected: mobxtree/UsersPermissionsUserConnectionUpdatedByModel.base.ts -> mobxtree/UsersPermissionsUserConnectionModel.ts -> mobxtree/UsersPermissionsUserConnectionModel.base.ts -> mobxtree/UsersPermissionsUserGroupByModel.ts -> mobxtree/UsersPermissionsUserGroupByModel.base.ts -> mobxtree/UsersPermissionsUserConnectionUpdatedByModel.ts -> mobxtree/UsersPermissionsUserConnectionUpdatedByModel.base.ts

using TS 3.7.3 and mst-gql 0.12.0

godness84 · 2020-07-28T17:06:32Z

@michaelspeed can you give more details? Does it compile? Does TS understand the types when you put the cursor on? Have you customized the code of any model? Maybe you introduced new circular deps?

Just looking at your few lines it's quite impossible to have any clues

michaelspeed · 2020-07-28T17:12:12Z

Here is the generated folder -
mobxtree.zip

and the schema -
schema.graphql.zip

michaelspeed · 2020-07-28T17:15:51Z

The error output looks like this -
error - ReferenceError: Cannot access 'UploadFileConnectionAlternativeTextModelBase' before initialization at Module.eval [as UploadFileConnectionAlternativeTextModelBase] (webpack-internal:///./mobxtree/UploadFileConnectionAlternativeTextModel.base.ts:2:136)

using Next js

chrisdrackett changed the title ~~'<XXX>ModelBase' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.~~ Circular dependencies between models Jul 2, 2019

chrisdrackett mentioned this issue Aug 2, 2019

"RootStoreBase implicitly has type 'any' ..." when generating models for a Hasura schema #72

Closed

mweststrate mentioned this issue Aug 2, 2019

Update Examples #69

Merged

chrisdrackett mentioned this issue Sep 25, 2019

Generate types #110

Closed

4 tasks

chrisdrackett closed this as completed Nov 22, 2019

Circular dependencies between models #45

Circular dependencies between models #45

Comments

chrisdrackett commented Jun 29, 2019 • edited Loading

Amareis commented Jul 5, 2019

Amareis commented Jul 5, 2019 • edited Loading

elie222 commented Jul 5, 2019

Amareis commented Jul 5, 2019

Amareis commented Jul 5, 2019

chrisdrackett commented Jul 5, 2019

elie222 commented Jul 11, 2019

Amareis commented Jul 12, 2019

mweststrate commented Jul 12, 2019 via email

mweststrate commented Jul 17, 2019

chrisdrackett commented Jul 26, 2019

elie222 commented Jul 26, 2019 via email

mweststrate commented Aug 2, 2019

beepsoft commented Sep 4, 2019 • edited Loading

elie222 commented Sep 4, 2019 via email

beepsoft commented Sep 4, 2019

elie222 commented Sep 4, 2019 via email

beepsoft commented Sep 4, 2019

beepsoft commented Sep 5, 2019

chrisdrackett commented Sep 5, 2019

beepsoft commented Sep 24, 2019

elie222 commented Sep 24, 2019 via email

beepsoft commented Sep 24, 2019

chrisdrackett commented Sep 24, 2019

chrisdrackett commented Sep 24, 2019

beepsoft commented Sep 25, 2019

RXminuS commented Sep 25, 2019

chrisdrackett commented Sep 25, 2019

chrisdrackett commented Sep 25, 2019

RXminuS commented Sep 30, 2019

beepsoft commented Sep 30, 2019

kpogodin commented Oct 1, 2019 • edited Loading

pvpshoot commented Oct 1, 2019

RXminuS commented Oct 1, 2019

RXminuS commented Oct 1, 2019

pvpshoot commented Oct 2, 2019

beepsoft commented Oct 10, 2019

onomated commented Oct 28, 2019 • edited Loading

beepsoft commented Oct 28, 2019

onomated commented Oct 28, 2019

beepsoft commented Oct 28, 2019

chrisdrackett commented Nov 22, 2019

beepsoft commented Nov 22, 2019 • edited Loading

godness84 commented Nov 22, 2019

michaelspeed commented Jul 28, 2020 • edited Loading

godness84 commented Jul 28, 2020

michaelspeed commented Jul 28, 2020

michaelspeed commented Jul 28, 2020 • edited Loading

chrisdrackett commented Jun 29, 2019 •

edited

Loading

Amareis commented Jul 5, 2019 •

edited

Loading

beepsoft commented Sep 4, 2019 •

edited

Loading

kpogodin commented Oct 1, 2019 •

edited

Loading

onomated commented Oct 28, 2019 •

edited

Loading

beepsoft commented Nov 22, 2019 •

edited

Loading

michaelspeed commented Jul 28, 2020 •

edited

Loading

michaelspeed commented Jul 28, 2020 •

edited

Loading