Fix input types resolver in generators

[zazido]

Fixes the potential unnecessary recursive function calls. Resolves #310

For a type structure like this:

A
├── B ── C 
└── D ── C

A has child types B & D, B and D both have C.

When executing deepResolveInputTypes(inputTypesMap, typeName, seen) on A -> B -> C -> D -> C, C will still be marked as unseen since seen is a new object on line:

https://github.com/prisma/graphqlgen/blob/f7a90f41aa297e70a1076698c9393096dab3ec12/packages/graphqlgen/src/generators/common.ts#L243

Instead, we should use seen itself.

TODO

• update inline snapshot of deepResolveInputTypes test in common.spec.ts.

[jasonkuhrt]

@zazido Thanks!

1. It would be great if we can get unit test(s) coverage. I could help. My plan is to move to co-located unit test modules, e.g.: /src/generators/common.spec.ts. I could get the ball rolling via a commit to master that you then rebase. Then you can focus just on the test and not the meta stuff around it. (done)

2. ✅ Also, it would be great if someone from When graphqlgen large schema will get stuck #310 such as @Victorkangsh @vdiaz1130 @woss (anyone from that thread!) could checkout this branch and test if if fixes their issue.
   see emoji reactions in Fix input types resolver in generators #373 (comment)

3. In addition to a unit test we should update the prisma schema fixture to exhibit this pattern. We should make a comment in the fixture to that effect, even linking to this PR for example.
   too much effort/maintenance for now

🙏

[jasonkuhrt]

Should have left my feedback in review form. See above.

[zazido]

@jasonkuhrt thanks for the great tool. I'm glad to help with 1 & 3 after your commit.

For those who want a quick fix from #310, go to node_modules/graphqlgen/dist/generators/common.js and change the function deepResolveInputTypes to:

function deepResolveInputTypes(inputTypesMap, typeName, seen) {
    if (seen === void 0) { seen = {}; }
    console.log(seen);
    var type = inputTypesMap[typeName];
    if (type) {
        var childTypes = type.fields
            .filter(function (t) { return t.type.isInput && !seen[t.type.name]; })
            .map(function (t) { return t.type.name; })
            .map(function (name) {
            seen[name] = true;
            return deepResolveInputTypes(inputTypesMap, name, seen);
        })
            .reduce(utils_1.flatten, []);
        return [typeName].concat(childTypes);
    }
    else {
        throw new Error("Input type " + typeName + " not found");
    }
}

and rerun node_modules/.bin/gg to see if there's any improvements.

Looking forward to your feedback.

[jasonkuhrt]

I will try to address my own points but not sure when I will have it done. If you can do it before me all the better : )

[jasonkuhrt]

What is the intended effect of your change, given:

import * as _ from 'lodash'

const seen = {}
const name = 'bar'

const seenCopy = { ...seen, [name]: true }
seen[name] = true

assert(_.isEqual(seen, seenCopy))

?

[jasonkuhrt]

Ah, but by using mutation to share state across the recursion's various branches of the tree-of-types, we effectively guarantee that said branches share their seen state... which otherwise is not the case (seen state is effectively forked at every node.

[jasonkuhrt]

@zazido done in 0199cca.

• Added TODO to your description.

• Seems to me this might be more accurate to call this patch a perf improvement (critical, for certain huge schemas)?

  fwiw/fyi this patch doesn't seem to dent existing prisma benchmark (this might be more a question of having the right fixture data, though):

  before:

  generateCode (prisma-complex schema, flow) x 1,224 ops/sec ±1.62% (87 runs sampled)
  generateCode (prisma-complex schema, typescript) x 1,822 ops/sec ±0.66% (94 runs sampled)    
  

  after:

  generateCode (prisma-complex schema, flow) x 1,232 ops/sec ±2.29% (88 runs sampled)
  generateCode (prisma-complex schema, typescript) x 1,884 ops/sec ±0.44% (96 runs sampled)
  

• I take back the integration test request, not enough ROI IMO