Internal Imports to Avoid Circular Dependencies

[RXminuS]

Breaking Change: any existing Models will need to either be re-generated or remove the export { selectFrom*, ... } at the beginning of the file.

Background

The nature of GraphQL is that it is very easy to end up with circular dependencies. Simply something as

type Node {
    edges: [Edge!]!
}

type Edge {
    from: Node!
    to: Node!
}

can quickly become problematic. mstGQL currently doesn't allow any elegant way of controlling the import order resulting in some funky behaviour in some cases. In our case, a particular ModelSelector was being imported before all the models that were required had been loaded resulting in some unclear error messages.

Solution

@mweststrate wrote an excellent blogpost with an elegant workaround that uses internal.js files.

This change simply makes sure that all the modules now use import {...} from "./internal" rather than individual files. It also generates an index.js/ts file that exports all the public modules. Finally by setting the keepInternalOrder flag the generator loads in the existing internal.js/ts file and preserves the order of any files in it. This allows you to re-order any of the modules giving the end-user the ability to resolve any circular dependencies.

Final Thoughts

Although I'm working on a big refactor using the Gluegun framework I think it would be a good idea to either settle or discard the idea of internal.js/ts module loading now so that we can maintain that style in future versions.

[RXminuS]

One problem with this method is that it's not working so great with --isolatedModules in Typescript. facebook/create-react-app#6054. I think the easiest solution would be to simply export the Model files as export * from "...Model"

[pvpshoot]

Ok, it turned out to be harder than it sounds.
again the module is imported as undefined.

Do you need some code for test cases?
this is my schema https://pastebin.com/czEpyrP6

[RXminuS]

@pvpshoot Can you try move all the "Enum's" to the top of the internal.js/ts file? (or just below BaseModel.ts/js)

[pvpshoot]

Yes, this solves the problem #117 (comment)

[pvpshoot]

can we merge it?

or can we help you?

[mweststrate]

LGTM. But note that I am not very on top of this project, for more in depth reviews, I recommend tagging @chrisdrackett :)

[RXminuS]

Ok, @pvpshoot this seems to be working properly now. Not sure how to merge but should be OK to do so

[chrisdrackett]

@RXminuS I don't see any updates to the readme. I'm assuming some are needed as part of this change?

[chrisdrackett]

@RXminuS also, do any or all of the examples need updating?

[RXminuS]

True, also I just realised...I'm still exporting a bunch of things as Type that are actually Interfaces. @pvpshoot , maybe you have time to look at that?

[chrisdrackett]

what does this option do?

[RXminuS]

It reads in the existing internal.js/s file and how things are ordered in it and then produces a new internal.js/ts file that tries to keep modules in that same order (prepending new ones).

This very easily allows you to resolve circular import dependencies. I've put it behind a flag so that it doesn't break for people not using the feature because suddenly it will prepend any new modules that are added later (which is almost certainly wrong since they get imported before BaseModel)

[chrisdrackett]

maybe I'm still confused, but because this is a generated file can't we just always import things in an ideal order and forgo adding an option?

[RXminuS]

It's similar to what I did in 1935ead

[pvpshoot]

@RXminuS i'm a noob in TS but i will try =)

[chrisdrackett]

@RXminuS is this solved by the recently released 0.7.0 or is this PR still valid?

[chrisdrackett]

@RXminuS going to close this for now, but if this is still valid please feel free to update and reopen!

[techknowfile]

I still encounter circular import issues during webpack in mst-gql v1.22.4