Open cjreimer opened 10 months ago
Josh-Walker-GM
commented
10 months ago 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.
cjreimer
commented
10 months ago 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!
dthyresson
commented
10 months ago Just a bump to assign to @Josh-Walker-GM to follow up.
Josh-Walker-GM
commented
10 months ago Hey @cjreimer, could you clarify IssueReducedResolverType for me? I think it'll help me reproduce this which I haven't yet
cjreimer
commented
10 months ago 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!
Josh-Walker-GM
commented
10 months ago 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.
cjreimer
commented
10 months ago @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!
Josh-Walker-GM
commented
9 months ago @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.
larsvaehrens
commented
2 months ago I just stumbled upon this exact issue. We ran into the same IntelliSense errors with type instantiation is excessively deep and possibly infinite some time ago, but didn't dig into it at the time. Now when trying to set up a CI workflow with yarn rw type-check api we get the out-of-memory issue too:
@cjreimer did you make a workaround for the circular type references? We could disable some of the relation resolvers to fix this issue, but I'd rather not go down that path if there is a better solution.
cjreimer
commented
2 months ago @larsvaehrens in the end we had to move forward on this and I took out some relations in the sdl files that we didn't really need. The mesh structure appears to work for graphQL, but the way the typescript structure is right now, it causes issues if there are too many loops. I spent a bit of time trying to figure out the exact conditions that causes this error, but I found it's quite finicky and complex.
Hope that helps.
larsvaehrens
commented
1 month ago Thank you for the suggested workaround.
I have been trying to figure out which part of our structure was causing issues by removing relations one by one but I got to a point where I could not remove any more relations from the SDLs without causing yarn rw g types to error. So I tried removing all relation resolvers from all service files and this fixed the out-of-memory issue on yarn rw type-check api. This was without changing any SDLs which I wasn't too fond of because that would mean the generated types would not be as expected.
What's not working?
Summary
Posted as an troubleshooting FYI for anyone else that could run into this...
As Redwood v6.5.0 now upgrades typescript to v5.3.2, the latest typescript releases appear to be susceptible to memory errors on types that can recurse infinitely. The key to solving these issues is to run:
yarn tsc --noEmit --extendedDiagnosticsfrom the api or web side folder as appropriate.This identified where the issues were in code.
From an architecture point of things, we got into this scenario when we had child graphQL types that referenced the parent. For example:
Details
We had memory heap limit issues when running Redwood
6.5.0. No issues were encountered on previous releases up tov6.4.2. We understand that typescript was upgraded tov5.3.2in Redwoodv6.5.0. We tried forcing a resolution to typescript patchv5.3.3, which is now out, with no change. We see the following error:Note that
yarn rw g typesworks fine. However,yarn tsc --skipLibCheckfrom theapifolder failed in the same way.Note that the previous redwood version had typescript pinned to
typescript: 5.2.2.This was looking like a problem in typescript
v5.3.xthat was specific to our codebase.What became interesting wasthe result we get when we ran the following:
I also ran this on the same codebase with the previous redwood version, and had the same errors, minus this one:
Cleaning up these two resolvers that could recurse solved the issue. The latest version of typescript doesn't seem to handle this case nicely! In the summary, I have provided some insights into how our code got into this scenario.
How do we reproduce the bug?
No response
What's your environment? (If it applies)
Are you interested in working on this?