Thanks so much for investigating this!

We should be careful not to mutate inputs but also careful to ensure the performance of query execution is not negatively impacted. One case I imagine would not work correctly here is if a query includes the same fragment in two locations, an error occurring in both locations. The parentField property would have been overridden by the second call, and the first error would have an incorrect execution path.

Add unittest for

One case I imagine would not work correctly here is if a query includes the same fragment in two locations, an error occurring in both locations.

Maybe I have misunderstanding?

And the problem of add parentField for AST, can you provide a better way to retain parent field information without modification of AST?

ping @leebyron

For example:

fragment X on T {
  x
}

query {
  a { ...X }
  b { ...X }
}

Where the field x throws an error.

Currently by my understanding of your code, this will result in printing out two errors, where both errors will report b.x even though the first should report a.x - this occurs because of the mutation of the field nodes, and the fragment being used in more than one location.

At the very least, this test should be included so that this is known not to occur.

My opinion on how to maintain the execution path is to maintain this as an additional "Stack" variable provided during execution, rather than mutating the AST provided to the execution.