Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot read property 'name' of undefined when merge requried and not #159

Closed
Fi1osof opened this issue Aug 31, 2018 · 6 comments
Closed
Labels
🐞bug Something isn't working 📦released This issue or pull request is released

Comments

@Fi1osof
Copy link
Contributor

Fi1osof commented Aug 31, 2018

Bug.

[email protected]

Merge two types:

input UserGroupUpdateInput {
  name: String
  Users: UserUpdateManyWithoutGroupsInput
}
input UserGroupUpdateInput {
  name: String!
}

(In first name not required, in second required).
Got error:

...server/node_modules/merge-graphql-schemas/dist/index.cjs.js:4312
      if (field.type.type.name.value !== original.type.type.name.value) {
                                                            ^

TypeError: Cannot read property 'name' of undefined
    at ...server/node_modules/merge-graphql-schemas/dist/index.cjs.js:4312:61
    at Array.reduce (<anonymous>)
    at _makeMergedFieldDefinitions (...server/node_modules/merge-graphql-schemas/dist/index.cjs.js:4301:46)
    at defs.filter.reduce.Query (...server/node_modules/merge-graphql-schemas/dist/index.cjs.js:4345:17)
    at Array.reduce (<anonymous>)
    at _makeMergedDefinitions (...server/node_modules/merge-graphql-schemas/dist/index.cjs.js:4331:6)
    at mergeTypes (...server/node_modules/merge-graphql-schemas/dist/index.cjs.js:4384:20)
    at CoreModule.getApiSchema (...server/src/node_modules/prisma-module/src/index.js:112:13)
    at CoreModule.getApiSchema (...server/src/modules/index.js:125:5)
    at generateSchema (...server/src/node_modules/react-cms-schema/src/index.js:48:27)

@RichardLitt RichardLitt added the 🐞bug Something isn't working label Sep 5, 2018
@cfnelson
Copy link
Contributor

@Fi1osof Thanks for reporting this, the intended behaviour in this case would be to throw an error alerting the user to conflicting types in the two files. Guessing this might have been missed for input types? A PR would be welcome.

@cfnelson cfnelson added the ⛑️help wanted Extra attention is needed label Sep 11, 2018
@Fi1osof
Copy link
Contributor Author

Fi1osof commented Sep 12, 2018

@cfnelson i think need not error alerting, just make result field required.

@cfnelson
Copy link
Contributor

@Fi1osof I think if there are any conflicts like the above example, they should be manually fixed, or a specific object is passed in as an option on how to resolve the multiple conflicting field type definitions into the specific type you specified.

e.g -> Something like below could be passed into the mergeTypes function as an option to resolve your original example. It would instruction the mergeTypes on which type field definintion to resolve/merge. This is assuming someone doesn't wish to manually fix their graphql.

const resolveConflicts = {
  UserGroupUpdateInput: "String!"
};

@Fi1osof
Copy link
Contributor Author

Fi1osof commented Nov 14, 2018

@cfnelson graphql allow set notrequired variable as required. For example:

input where: String

query test($where: String!){
    docs(where: $where)
}

But can not like

input where: String!

query test($where: String){
    docs(where: $where)
}

So via merge schema have ability set notrequired field as required.
input where: String
input where: String!
became input where: String!

@kamilkisiela
Copy link
Collaborator

Should be fixed by ardatan/graphql-toolkit#189

@kamilkisiela kamilkisiela added ⏳waiting for release This issue or pull request is waiting to be released and removed ⛑️help wanted Extra attention is needed labels Jul 31, 2019
@kamilkisiela
Copy link
Collaborator

Fixed with v1.7.0

@kamilkisiela kamilkisiela added 📦released This issue or pull request is released and removed ⏳waiting for release This issue or pull request is waiting to be released labels Jul 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞bug Something isn't working 📦released This issue or pull request is released
Projects
None yet
Development

No branches or pull requests

4 participants