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

Fix buggy implementation of add query functionality #132

Closed
imolorhe opened this issue Nov 7, 2017 · 2 comments
Closed

Fix buggy implementation of add query functionality #132

imolorhe opened this issue Nov 7, 2017 · 2 comments

Comments

@imolorhe
Copy link
Collaborator

imolorhe commented Nov 7, 2017

The add query functionality does not work and hangs the application in certain conditions.
This should be addressed.

Expected Behavior

Adding a query should always work, else the button shouldn't be there.

Current Behavior

For some graphQL servers, when you click the "Add Query" button, the application hangs.

Possible Solution

The likely culprit is because of an infinite recursive loop. If the recursion is unending, a maximum depth limit can be set. It is not likely that a user would want to add a query with more than 50 (random number) depth fields, for instance.

Steps to Reproduce (for bugs)

  1. Enter a valid graphQL server URL with introspection enabled.
  2. Open the docs
  3. Click the "Add Query" button that appears when you hover over a query.

Context

Your Environment

  • Version used:
  • Environment name and version (e.g. Chrome 39, node.js 5.4):
  • Operating System and version (desktop or mobile):
  • Link to your project:
@imolorhe
Copy link
Collaborator Author

imolorhe commented Nov 7, 2017

It turns out the problem comes from codemirror trying to lint the query added to the editor. That might not be the only reason but looking through the call stack, it appears the recursive functions completed successfully and returned the data to codemirror. For now, I will set the depth limit to a conderable number (3) and would later add the ability to allow the user to modify the depth level, with a warning (of course) about the repercussion of doing that.

@imolorhe
Copy link
Collaborator Author

imolorhe commented Nov 7, 2017

It turns out in some cases, that the query added even with a depth limit of 3 is still too much for codemirror linting to handle. Weird. I might have to disable the feature depending on the size of the index.

imolorhe pushed a commit that referenced this issue Nov 7, 2017
for now.
Should try to increase it a little more for the user.
Another approach would be to make a decision on the depth level based on
the size of the search index.
@imolorhe imolorhe closed this as completed Nov 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant