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

Add terminateCircularRelationships option #31

Conversation

ford-outreach
Copy link
Contributor

I ran into #13 when trying to bring in this plugin.

This introduces an option, terminateCircularRelationships, that when enabled, allows a particular type to only be resolved once by the mocks in a particular call stack. After the first time, subsequent resolutions will return an empty object.

This does not affect overrides, so if you need a circular structure, it can still be achieved using overrides.

Testing was a little tricky, because this issue doesn't present itself until you actually call one of the generated mock functions.
What I've done is created a script (tests/circular-mocks/create-mocks.ts) that generates a set of mock functions for a schema with circular relationships. I hooked it up as a jest global set up script, so it will run before any test runs.

Then added a new test that imports the generated file and runs the mock functions (tests/circular-mocks/spec.ts)

I also pulled the code into our codebase, turned on the option, and it resolved the infinite recursion.

Copy link
Owner

@ardeois ardeois left a comment

Choose a reason for hiding this comment

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

Very nice thank you for the contribution !

I'll try to test it tomorrow before merging

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "graphql-codegen-typescript-mock-data",
"version": "1.2.1",
"version": "1.3.0",
Copy link
Owner

Choose a reason for hiding this comment

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

don't bump the version yourself, the CI will handle that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah cool, reverted that change

@ford-outreach
Copy link
Contributor Author

Thank you for the quick review! That's awesome

@ford-outreach
Copy link
Contributor Author

@ardeois Any chance this could get merged and released this week? I'd like to be able to pull it in directly rather than have to use a fork in the interim

@ardeois
Copy link
Owner

ardeois commented Oct 13, 2020

Yes thanks for the reminder, I'll check it today

@ardeois ardeois added the minor Increment the minor version when merged label Oct 13, 2020
@ardeois ardeois merged commit 070bea8 into ardeois:master Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants