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

Error: Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ #3

Closed
loretoparisi opened this issue Oct 28, 2016 · 20 comments
Closed

Error: Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ #3

loretoparisi opened this issue Oct 28, 2016 · 20 comments

Comments

@loretoparisi
Copy link

I'm trying to run against this schema from Musixmatch SDK

but I get

$ node app.js 
Error: Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "get_album.tracks.get_message_header" does not.
    at invariant (/tools/swagger-to-graphql/node_modules/graphql/jsutils/invariant.js:19:11)
    at assertValidName (/tools/swagger-to-graphql/node_modules/graphql/utilities/assertValidName.js:28:27)
    at new GraphQLObjectType (/tools/swagger-to-graphql/node_modules/graphql/type/definition.js:224:42)
    at createGQLObject (/tools/swagger-to-graphql/lib/type_map.js:68:10)
    at jsonSchemaTypeToGraphQL (/tools/swagger-to-graphql/lib/type_map.js:94:12)
    at _.mapValues (/tools/swagger-to-graphql/lib/type_map.js:79:13)
    at /tools/swagger-to-graphql/node_modules/lodash/lodash.js:13303:38
    at /tools/swagger-to-graphql/node_modules/lodash/lodash.js:4917:15
    at baseForOwn (/tools/swagger-to-graphql/node_modules/lodash/lodash.js:2979:24)
    at Function.mapValues (/tools/swagger-to-graphql/node_modules/lodash/lodash.js:13302:7)
    at getTypeFields (/tools/swagger-to-graphql/lib/type_map.js:76:20)
    at createGQLObject (/tools/swagger-to-graphql/lib/type_map.js:71:13)
    at jsonSchemaTypeToGraphQL (/tools/swagger-to-graphql/lib/type_map.js:94:12)
    at _.mapValues (/tools/swagger-to-graphql/lib/type_map.js:79:13)
    at /tools/swagger-to-graphql/node_modules/lodash/lodash.js:13303:38
    at /tools/swagger-to-graphql/node_modules/lodash/lodash.js:4917:15
    at baseForOwn (/tools/swagger-to-graphql/node_modules/lodash/lodash.js:2979:24)
    at Function.mapValues (/tools/swagger-to-graphql/node_modules/lodash/lodash.js:13302:7)
    at getTypeFields (/tools/swagger-to-graphql/lib/type_map.js:76:20)
    at createGQLObject (/tools/swagger-to-graphql/lib/type_map.js:71:13)
    at Object.keys.filter.reduce (/tools/swagger-to-graphql/lib/index.js:59:18)
    at Array.reduce (native)

Thank you.

@loretoparisi
Copy link
Author

[UPDATE]
Digging around it seems that it's a convention of GraphQL names that I do not have as counterpart in Swagger, that is validated in assertValidName.js regex

var NAME_RX = /^[_a-zA-Z][_a-zA-Z0-9]*$/;

In fact that is the error message

function assertValidName(name) {
  (0, _invariant2.default)(NAME_RX.test(name), 'Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "' + name + '" does not.');
}

So, any idea how to get rid of this - without changing the swagger (and all autogen code)?

@yarax
Copy link
Owner

yarax commented Oct 29, 2016

Hi, thanks for the good issue!
I changed the regexp and checked everything against your schema, should work

@loretoparisi
Copy link
Author

loretoparisi commented Oct 31, 2016

@yarax You are welcome, thank you it is fixed now (no errors at node app, didn't try the api yet). A question, I have tried to serialize the schema object of graphqlHTTP into a string using JSON.stringify, but it seems it has like a circular structure. Is this the expected behaviour?

I have tried util.inspect( schema ) to search for [Circular] replacements (as it should emit in these cases) as well, but the string output seems to be not a valid json...

Thank you.

@yarax
Copy link
Owner

yarax commented Oct 31, 2016

Yes, types can contain circular data structures (unlike Input Type), but in the result schema they are all together

@loretoparisi
Copy link
Author

@yarax ah so! And how to serialize / deserialize this schema? With your tool I can serve a REST api from swagger to GraphQL and it's amazing, but supposed that I want to serialize the scheme in order to provide it to 3rdparty, how do to that?

Thanks!

@yarax
Copy link
Owner

yarax commented Nov 2, 2016

It's not possible yet, but soon will be released a version compatible with https://github.com/nodkz/graphql-compose and then it will be possible to generate pure GraphQL types with its help from schema. #2

@yarax
Copy link
Owner

yarax commented Nov 2, 2016

But it sounds a bit strange to give types for 3rdparty because types are coupled with resolvers and resolvers should be implemented into your code base.

@loretoparisi
Copy link
Author

@yarax I got it, I was considering graphql as a sort of swagger api definition, but is not properly. If you see here https://github.com/musixmatch/musixmatch-sdk I release our API SDKs through the Swagger definition, so my idea was to have something like that for graphql.

@yarax
Copy link
Owner

yarax commented Nov 2, 2016

So I would probably advice you to share GraphiQL interface for your API clients, like github has done it https://developer.github.com/early-access/graphql/guides/accessing-graphql/

@yarax yarax closed this as completed Nov 4, 2016
@loretoparisi
Copy link
Author

@yarax yes that is the right way. Thank you for the fix.

@loretoparisi
Copy link
Author

@yarax sorry one more question, when running the schema from app.js I get

{
  "errors": [
    {
      "message": "Schema must be an instance of GraphQLSchema. Also ensure that there are not multiple versions of GraphQL installed in your node_modules directory."
    }
  ]
}

in the graphiql UI.

@yarax
Copy link
Owner

yarax commented Nov 4, 2016

Just have run fresh swagger-to-graphql against your schema (just replaced default petstore.json) https://playground.musixmatch.com/swagger.json and able to make queries http://storage2.static.itmages.com/i/16/1104/h_1478268736_3020583_41fa51db8e.png

@yarax
Copy link
Owner

yarax commented Nov 4, 2016

try maybe to reinstall node modules
rm -rf node_modules/* && npm i

@sibelius
Copy link
Contributor

sibelius commented Jun 1, 2017

@yarax I have the same issue, can we use something like this https://github.com/sindresorhus/camelcase to make sure all generate GraphQL code is valid?

@0xR
Copy link
Collaborator

0xR commented May 18, 2018

I'm experiencing the same issue because some request headers contain a - character

@fgcui1204
Copy link

@0xR I meet the same issue. Do you fix that?

@yarax
Copy link
Owner

yarax commented Jun 12, 2018

@fgcui1204 which version do you use? Should be fixed already

@0xR
Copy link
Collaborator

0xR commented Jun 12, 2018

The PR #75 was not merged yet. I'll have a look to see if the tests pass.

@0xR
Copy link
Collaborator

0xR commented Jun 12, 2018

@fgcui1204 I merged #75, can you try the latest master of swagger-to-graphql by cloning it into your node_modules? (I've had issues with npm link before)

@fgcui1204
Copy link

@0xR Thanks very much

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants