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

Enum no longer work with graphql master branch #761

Closed
grenierdev opened this issue Apr 29, 2018 · 3 comments
Closed

Enum no longer work with graphql master branch #761

grenierdev opened this issue Apr 29, 2018 · 3 comments
Assignees

Comments

@grenierdev
Copy link

How to reproduce

# Build graphql from master
git clone https://github.com/graphql/graphql-js.git
cd graphql-js
npm i
npm run build
mv dist ../graphql
cd ../graphql
npm i
npm link
cd ..

# Clone graphql-tools and link graphql we just built
git clone https://github.com/apollographql/graphql-tools.git
cd graphql-tools
npm i
npm link graphql
npm run test

Investigation

Seems like a commit made in March broke how enum values are cached within GraphQLEnumType.

After this commit, values are stored in a _valueLookup map in the constructor. So the following code is now broken as the lookup Map is not updated :
https://github.com/apollographql/graphql-tools/blob/4261e1ced8a0b9c31b2e2116cdf13f4d560194af/src/schemaGenerator.ts#L451-L452

Possible solution

The following monky patch does solve the problem.

const fieldValue = type.getValue(fieldName);
const fieldValueOld = fieldValue['value'];
const fieldValueNew = resolvers[typeName][fieldName];
(type as any)._valueLookup.delete(fieldValueOld);
(type as any)._valueLookup.set(fieldValueNew, fieldValue);
fieldValue['value'] = fieldValueNew;

graphql-js project does not expose a public API to update the internal lookup map, thus the cast to any to bypass typescript type checking.

Conclusion

Is this kind of patch acceptable ?

@DivXPro
Copy link

DivXPro commented May 26, 2018

same issue

@hwillson
Copy link
Contributor

hwillson commented Sep 7, 2018

Just dropping a reference - we're addressing this as we work through #945.

@hwillson hwillson self-assigned this Sep 7, 2018
@yaacovCR
Copy link
Collaborator

You can use toConfig to do all this much more reality easily in graphql-tools@next, see #1306

@yaacovCR yaacovCR mentioned this issue Mar 30, 2020
22 tasks
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

4 participants