review function sanitizeEnumValue()

[wwmoraes]

As per PR #637, the function sanitizeEnumValue() does not comply with the GraphQL documentation:

• Expects string-only values (GraphQL API declares value?: any)
  • GraphQL accepts (and states as "often") number values, which causes a TypeError on value.trim()
• sanitization of numeric-started strings result in underscore-prefixed values ('1' is sanitized to '_1')
  • sequelize simply casts to string on schema, thus making those underscored values invalid

I didn't found any references on the docs for why number/numeric-started string values need an underscore prefix.

[mickhansen]

https://github.com/mickhansen/graphql-sequelize/pull/502

[wwmoraes]

502

Great! Now I can play again with numeric enum values 🥇

[mickhansen]

@wwmoraes Huh? I was mostly referring to the PR that introduced this issue.

[wwmoraes]

@wwmoraes Huh? I was mostly referring to the PR that introduced this issue.

Oh, my bad. The PR's title is misleading.

[wwmoraes]

Ok so just documenting here before any coding: the PR #502 does state that GraphQL enum types "only support ASCII alphanumeric characters, digits and underscores with leading non-digit" which is right, but it doesn't mention that, in GraphQL, there's no enum value: the name is the value itself.

Naming convention (first/second/latest spec [and all in between]):

Name :       */[_A-Za-z][_0-9A-Za-z]/**

Enum values (first/second/latest spec):

• First spec

  EnumValue :       Name

• Second and latest spec [and all in between]:

  EnumValue :       Name but not true or false or null

Even though JS doesn't have an enum type, solutions like Object.freeze()'ing a key-value objects (being the values strings, numbers or even better, Symbol()) simulate the same enums saw in most languages. Thus, graphql-sequelize should transcript only the enum "name" to GraphQL, making the bridge between the name and value in sequelize queries.

[mickhansen]

@wwmoraes Mind working on a PR? Then we can discuss the changes and cut it as a major

[wwmoraes]

@mickhansen sure thing!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.