[Dependency Bug?]: V6.5.0 type-check on API side causing memory issues with types that can recurse about redwood HOT 8 OPEN

Just a bump to assign to @Josh-Walker-GM to follow up.

from redwood.

Thanks for the detailed issue @cjreimer! I'll reproduce this tomorrow locally and have a read through the related issues on the TS repo. Basically the next step here is on me to reproduce. We have been releasing at speed lately so hopefully if TS has a further patch out that fixes this behaviour we'll get it in relatively soon.

from redwood.

Thanks @Josh-Walker-GM. Note that we did have some custom types on the resolvers to try to get around the recursive issues. If you are trying to reproduce, then the below might help show what we were doing (with cleanout of non-relevant code). We weren't actually using this feature on the client / web side, so we are planning to blow away the recursive part of this in our graphQL schema. That being said, it's easy enough for someone to get into this case!

//=======================================
// Resolver
//=======================================

type IssueActionRelationResolversModified = Omit<
  IssueActionRelationResolvers,
  'issue'
> & {
  issue: RequiredResolverFn<
    IssueReducedResolverType,
    ResolversParentTypes['IssueAction'],
    RedwoodGraphQLContext
  >
}

export const IssueAction: IssueActionRelationResolversModified = {
  issue: (_obj, { root }) => {
    if ('issue' in root) return root.issue as ResolversTypes['Issue']
    return db.issueAction.findUnique({ where: { id: root.id } }).issue()
  },
}

Thanks!

from redwood.

Hey @cjreimer, could you clarify IssueReducedResolverType for me? I think it'll help me reproduce this which I haven't yet

from redwood.

Josh, I spent a good amount of time looking into this further last night and this morning. I'm pretty sure this is not related to the custom typing we did ... that's another topic. When I removed the custom typing, the issue remains. We have a scenario in our graphQL schema that can cause typescript to try to analyze type instantiation that is excessively and possibly infinite. See the error we get in vscode in the image below.

What is interesting is that we do not get this error in yarn rw type-check or when running tsc directly, so it appears that this error is not spit out upon compilation. I'm not sure how long we had this error, but it may have been there a few weeks to a month without us noticing, due to yarn rw type-check not showing it.

There is something quite tricky going on to get into this case, based on when we get this warning. As you know, graphql is well ... a graph or mesh, and our schema interlinks quite a bit with many relation edges. In playing with it, trying to add and remove edges, there appears to be a fairly complex configuration required to generate this error, not just a simple node that points back to its parent node. I think multi-loops are required, but I haven't had the time to figure out the exact pattern that results in this error.

So, it looks like we have a complex scenario in our graphQL schema that causes an infinite type scenario in graphQL-codegen and can break typescript. I think for you or the graphQL-codegen to look at this further, we'll need to map out a minimum graphql schema configuration that can generate this scenario. Leave that with us. As for us, in the moment, we can solve this for the moment by taking out a relation edge or two.

Thanks!

from redwood.

Thanks again @cjreimer! I agree that it might be helpful to find a schema of sufficient complexity to trigger this. I did try with some basic recursive types but I suspected it wasn't so complex as to reproduce this issue. I'll leave it with you but feel free to ping me again.

from redwood.

@Josh-Walker-GM , ok, I spent some time playing with our models, and it's not straight-forward. Here's a graph of some of our models and relations, and the red lines are the relations where if I break any one of them, we no longer get the type instantiation is excessively deep and possibly infinite warning. I didn't check every relation... this already burnt more time than I could really afford.

Now that we understand what's going on, this is relatively easy for us to fix for now, as we didn't need all these relations in the graphQL. That being said, I can see how some other decent-sized projects could get into this as well. There is a reasonable amount of discussion on this warning at:
https://github.com/i18next/react-i18next/issues?q=is%3Aissue+Type+instantiation+is+excessively+deep+and+possibly+infinite

I'm not sure how helpful this is for you, but it reinforces that we only get into this situation with a reasonably complex recursion scenario that has some depth to it.

As for recommendations for the redwood team, it would be very helpful, at minimum, if we could get the redwood yarn rw type-check command to spit out an error or warning on ts 2589. The fact we could only see this warning via IntelliSense made this whole scenario a head-scratcher for a while.

Thanks!

from redwood.

@cjreimer Thanks. I'll bring up enhancing yarn rw type-check with the team and get thoughts on it - seems reasonable to me. I'll follow up with a PR in that case or get back to you here if there are other ideas.

from redwood.