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

Use graphql-js builtin function separateOperations #179

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

detrohutt
Copy link
Contributor

This is the implementation of the changes discussed in #173.

The loader's size/complexity was greatly reduced by GraphQL's builtin function.
I also refactored the code structure/syntax to make it easier to reason about.
Mostly this just involved updating the order things were added to definitions.
I had to mess around a bit with the `require` mocking to facilitate new code.
I also unmocked require after each use, which was not being done.
assert.equal(Q1[0].name.value, 'Q1');
assert.equal(Q1[1].name.value, 'F33');
assert.equal(Q1[2].name.value, 'F22');
assert.equal(Q1[3].name.value, 'F11');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jnwng This pattern is brittle (hence me needing to change the tests). The order shouldn't matter IMO. I stuck with the same pattern for now, but if you'd like I can add a commit to change it. I'd map the names to an array and then do an assert.include for each value.

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

Successfully merging this pull request may close these issues.

None yet

1 participant