search

graphql / graphiql

GraphiQL & the GraphQL LSP Reference Ecosystem for building browser & IDE tools.
MIT License
16.1k stars 1.72k forks source link

[graphql-language-service-interface] add suggestions to spread fragments on Field #880

Open pieh opened 5 years ago

pieh commented 5 years ago

Right now fragment spread suggestions are shown only when you start with ... fragment spread operator.

I'd like to propose adding fragment spread suggestion earlier (on Field, SelectionSet etc), so we it's much easier for new GraphQL users to discover them.

Basic idea is to have it look something like: Screenshot 2019-05-26 at 15 30 07

Another suggestion from me would be to consider adding inline fragments to union types as well: Screenshot 2019-05-26 at 15 30 16 (unions and inline fragments are quite common pain points for Gatsby users that are new to GraphQL)

This would require just slight refactoring to reuse already existing code and I'm happy to submit pull request if this change would be welcome here.

orta commented 5 years ago

I'd like to see this 👍

benjie commented 5 years ago

Seconded 👍

pieh commented 5 years ago

That's awesome to hear, I'll try to find time to submit pull request this week

acao commented 5 years ago

So right now getAutocompleteSuggestions returns getSuggestionsForFieldNames, but you'll want to create a function that combines the latter with getSuggestionsForFragmentSpread. if you need help with the setup/linkages to test this end to end in graphiql let me know

pieh commented 5 years ago

if you need help with the setup/linkages to test this end to end in graphiql let me know

Thanks for offer. I already have setup for it and part of code changes (probably will need to rename some functions to make more sense, refactor bit more), but I'm still exploring what (IMO) could make sense to add there.

My current changeset: https://github.com/graphql/graphql-language-service/compare/master...pieh:feat/suggest-fragment-spreads

acao commented 5 years ago

Looks good! getFragmentsForType, good approach. To note, I don't think any of us maintainers here in this thread have the keys to npm currently, but I think @lostplan might

lostplan commented 5 years ago

@acao I do indeed have the keys to npm. (Am I the only one right now?! 😬We should definitely get that fixed)

acao commented 5 years ago

@lostplan oh that's great news! GraphQL Foundation is chasing that down as we speak. Do you happen to have the keys for codemirror-graphql as well?

lostplan commented 5 years ago

I should clarify… I am just a collaborator of graphql-language-service; I don't hold the master key. I don't think I have have permission to publish other repos.

acao commented 5 years ago

@lostplan that's very helpful still! will you be at the meeting tomorrow? This repo will be one of the main topics of discussion. Yes, we are hoping to rename it to the GraphQL IDE working group.

acao commented 5 years ago

@pieh feel free to open a PR in the new monorepo. changing the origin to this repo and rebasing might work, as the filename indexes for gls should be intact