graphql / graphql-js

A reference implementation of GraphQL for JavaScript
http://graphql.org/graphql-js/
MIT License
20.1k stars 2.03k forks source link

error in typeMapReducer causes graphql to fail on certain schemas #2439

Closed narration-sd closed 4 years ago

narration-sd commented 4 years ago

This error exists in both 14.x and 15.x branches, and has a one-line tested fix, so presenting that here rather than in multiple pull requests.

Issue:

In either branch latest, graphql fails immediately for certain schemas, reporting:
Error: Schema must contain uniquely named types but contains multiple types named "Name" where for my schema Name may be PageInfo or Mutation.

Others have reported similar problems, also for graphql as used in Gridsome.

Response:

Tracking this down via the stack trace and logging, the problem is that function typeMapReducer() in type/schema.js is doing a compare of objects for the Type name, to determine failure.

  if (seenType) {
    if (seenType !== namedType) {
      throw new Error("Schema must contain uniquely named types but contains multiple types named \"".concat(namedType.name, "\"."));
    }
...

Adding a cast to String before comparing solves this. The code becomes:

  if (seenType) {
    if (String(seenType) !== String(namedType)) {
      throw new Error("Schema must contain uniquely named types but contains multiple types named \"".concat(namedType.name, "\"."));
    }
...

This code appears at apparent lines 338 in type/schema.js source for 14.x, and 319 in 15.x source, as these appear on this github repo.

It seems both branches should be repaired with fresh release, especially14.x, as many are using that in the field. Thanks.

Rationale:

The commonality for the PageInfo and Mutation terms that fail with present code is that they appear in multiple graphql Connections.

It's possible that the uniqueness test really does intend to compare objects, but that the situation is complicated by the fact that in Gridsome (also potentially in Gatsby, perhaps others), there are data-source-resolving prefixes added to the actual declaration of some terms, which arrive early in the map array for typeMapReducer().

I'm attaching a log of that map array, as in use at the point where the failure occurs. so you can see the use of prefixes.

If the prefixes are the problem, then this 'fix' becomes a workaround, until a proper design can accomodate.

map.zip

narration-sd commented 4 years ago

...reflected a little more on what seems to be intended by typeMapReducer's check, so updated the issue statement, if you read early by email.

IvanGoncharov commented 4 years ago

reflected a little more on what seems to be intended by typeMapReducer's check, so updated the issue statement, if you read early by email.

@narration-sd Yes, you absolutely correct it's intentional. String(seenType) !== String(namedType) will never be false since seenType is found by name. This error typically happens when you have duplicating packages in your node_modules please ensure that npm ls graphql-relay returns only one package.

narration-sd commented 4 years ago

Ivan, thanks indeed for coming back so quickly.

I checked for duplicating packages; graphql-relay isn't present at all, and for alll other graphql*, the only indications of duplication were apparently references in the build parts of other packages: no actual duplicate module present so far as I could see, save one I'd already removed of graphql itself within the gridsome module.

So, I dumped out more of what typeMapReducer() is dealing with as initialTypes -- and, the initiating schema from the server, at least a reasonable facsimile.

So, I think the problem actually has to be on the Gridsome side, and perhaps specifically in some operations that are done in its connector to over-the-wire GraphQL servers. They're from a standardd library, but maybe there's something about how the calls are being applied that's allowing the doubling up. A clue for that is maybe finding the duplicate types near the end of agr.introspect.json; we'll have to see.

If you have any ideas, most welcome of course.

Again, much appreciation for your clear, immediate, and helpful response, Ivan.

Best, Clive

initialTypes.last.zip agr.introspect.zip full-map.zip

IvanGoncharov commented 4 years ago

@narration-sd If you can modify source code of graphql package you can add this line:

class GraphQLObjectType {
  constructor(...) {
    ...
    this.stackTrace = String(new Error().stack);
    ...
  }
}

and print stackTrace of conflicting types it will give you a clue where they were contstructed. If it's not duplicating packages the second most popular cause is when you try to do schema transformation and don't properly update types.

narration-sd commented 4 years ago

Thanks - that output's quite useful. Hadn't realized a stack trace could be had so easily...

A good weekend to you, spasibo

narration-sd commented 4 years ago

@IvanGoncharov , this is all sorted now, and reported for a close, over at the Gridsome issue.

It turns out I had an extra source added, in some well-hidden outdated configuration for an early, superseded plugin someone once built for them. This managed to merge the same remote schema, accurately, but without the prefixing, thus your preflight was quite correct in picking off the problem.

The stack traces were essential, in getting orders right, and thus arriving at this.

sibelius commented 2 years ago

how did you figure it out?