Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

explore combining all .base files into a single file #260

Open
chrisdrackett opened this issue Jul 28, 2020 · 9 comments
Open

explore combining all .base files into a single file #260

chrisdrackett opened this issue Jul 28, 2020 · 9 comments
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@chrisdrackett
Copy link
Collaborator

as a way to possibly avoid some circular import issues I wonder if it would be valuable to put all the generated code into a single file.

@chrisdrackett chrisdrackett added help wanted Extra attention is needed good first issue Good for newcomers labels Jul 28, 2020
@RXminuS
Copy link
Collaborator

RXminuS commented Jul 30, 2020

I can’t find the thread but there was a discussion about this earlier and I absolutely think it’s the way to go. I have some of it implemented on a branch and wasn’t too hard to do; but I got stuck on the following:

I wanted to figure out if we could somehow more organically allow extensions of a shared base models. So that you didn’t need all the non base files either and you just have a “mstgql.ts” file. Instead of importing each extended class in the base models I wanted to see if we could invert the logic somehow by using a explicit composition file that still allowed for proper Typescript types to propagate.

To do this rather than inferring the types I had to generate explicit interfaces for everything and this was a bit more work that I never completed. Not was I 100% convinced by my strategy yet.

If someone’s interested in pairing on this with me please give me a ping. I think getting it right will make mstgql feel a lot more intuitive to use without so many files. And no more circular import problems!

@RXminuS
Copy link
Collaborator

RXminuS commented Jul 30, 2020

This was the original thread:
#119

And lots of related issues
#190

@RXminuS
Copy link
Collaborator

RXminuS commented Jul 30, 2020

@Matth10 sorry, I’ve been of the scene a bit so missed your comments on those threads. Would you be interested or have someone on your team interested in pairing on this?

@chrisdrackett
Copy link
Collaborator Author

@RXminuS Maybe we should open up a new issue or project to cover the exploratory work around this idea? I think its good to keep this issue small and focused, but I'll all for simplification and eliminating the circular issues if at all possible.

@jvcmanke
Copy link

jvcmanke commented Aug 20, 2020

I second this!

Not even for the circular dependencies (even though it should be the main reason to do this), but for organization purposes as well. Even with a small schema I ended up with so many files that it made me get lost often.

I would love to even a have a separate types.d.ts and enums.ts files or something. I think it could go hand-in-hand with this exploration.

@jvcmanke
Copy link

I am open to help as well! I don't have that much time, but I really like the library and want to help where possible.

I took a look at the generation code, but since I'm not very familiar with it, I didn't understand it very much.

@special-character
Copy link
Contributor

I think this would be a good idea.

I used madge to look at why I had some circular dependencies (causing undefined imports) and found that there were tons of cycles just from base files importing other base files. For my case, a large majority of the cycles were from mst-gql models (like 70 circular deps). This make the code base feel really brittle to me.

It feels extra difficult to solve these because they are in generated files that I can't change manually to fix. I definitely think we could/should solve these when scaffolding.

@techknowfile
Copy link

techknowfile commented Jan 19, 2021

I'm also facing circular import problems with js files. Requires manually editing the base files to move the Selectors, select*() functions, and primitives to a separate file for many models, all of which gets undone when I re-scaffold

@Aryk
Copy link
Contributor

Aryk commented Sep 7, 2023

Just wanted to resurface this issue with more data (and a proposed workaround for now):

facebook/react-native#34476 (comment)

^^ TLDR; Even if you ignore the warnings, they will still show in your console now.

Potential fix (maybe helpful for other people like me, trying to figure out how to remove from my console on newer versions of Expo):

expo/expo#17773 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

7 participants