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

Path to GraphiQL v2 #2328

Closed
1 of 7 tasks
timsuchanek opened this issue Apr 19, 2022 · 26 comments
Closed
1 of 7 tasks

Path to GraphiQL v2 #2328

timsuchanek opened this issue Apr 19, 2022 · 26 comments
Labels

Comments

@timsuchanek
Copy link
Member

timsuchanek commented Apr 19, 2022

We have ambitious plans for the future of GraphiQL and want to go there step by step.

This is the path I suggest to move forward:

1. Implement new design with existing components

2. Modernize components step by step

As the new design has a slightly different layout, in Step 1, the components can already be modernized as needed.
If necessary, we'll create a separate issue for tracking work that's left.

3. Introduce monaco

4. Implement the new sidebar.

The new sidebar is quite ambitious, that's why we want to work on it last.

image

Additional tasks

While working on this, we want to revisit and consolidate the boards @acao created:

Some of the issues will be fixed by our tasks above, some are additional improvements, bug fixes, and features.

We'll have @thomasheyenbrock from GraphCDN focus on steps 1-4 in the coming weeks.
Anyone who's interested in collaborating, please let us know and we can organize code pairing sessions!

We also commit to 1h of issue triaging per week, as some of them grew a bit old. We start May 2nd at 4 pm UTC. Let me know if you want to join. Calendar link:

https://calendar.google.com/calendar/u/0/embed?src=linuxfoundation.org_ik79t9uuj2p32i3r203dgv5mo8@group.calendar.google.com

You can either answer here or join the graphiql Discord channel.

@timsuchanek timsuchanek pinned this issue Apr 19, 2022
@timsuchanek
Copy link
Member Author

See for reference #2122, which also tracks complementary issues

@acao
Copy link
Member

acao commented Apr 19, 2022

@timsuchanek the github project tabs are just filters based on label, you can remove them readily but then you'll have a huuuge list if that's ok! the main goal was to separate out monaco-graphql related work so as not to scare away folks who just want to work on the react side of things.

there are still some final things we need to add for monaco-graphql, such as providing API options so graphiql can navigate to the right doc entry when clicking on keywords (for example, click on a field and watch!), and similarly for hover and completion docs, so that we can have feature parity. There is a user defined spec for this floating around in our issues.

@timsuchanek
Copy link
Member Author

Awesome, thanks for the context, we'll have a look into those tasks once we tackle any monaco-graphql related things. I'll also ping you again before working on it just to make sure we don't miss anything, as you have most of the context.

@acao
Copy link
Member

acao commented Apr 19, 2022

Oh perhaps you missed the discussion on discord where I will be working on monaco editor adoption in parallel!

i was just commenting on this:

While working on this, we want to revisit and consolidate the boards @acao created:

@acao acao changed the title Path to v2 Path to GraphiQL v2 Apr 19, 2022
@n1ru4l
Copy link
Collaborator

n1ru4l commented Apr 22, 2022

How will this path be represented in releases?
When I think that each step could be an individual release step 1 would already be a huge breaking change if shipped 🤔 .

The following order seems to make more sense to me:

  1. Modernize components step by step (new minor)
    1.1. move to hooks
    1.2. use context API where it makes sense
  2. Introduce Monaco (new minor as there are no APIs for directly hooking into codemirror AFAIK)

Entering breaking change land

  1. Implement new design with existing components (major - alongside with this release it might then also make sense to revisit GraphiQL props and adjust the API)
  2. Implement the new sidebar (new minor)

@acao
Copy link
Member

acao commented Apr 22, 2022

this makes sense to me! this is almost the exact same roadmap we took in 2020 and it worked out great. the only mistake we made is that we didn't release incrementally as you indicate

we had most of the components converted to hooks and context two years ago, so that should be a great point of reference for anyone who wants to embark on this again! Just ignore the monaco specifics, and localisation, and take into account all the functionality we've added since

the move to context allowed us to seperate out the business logic into context providers and react useReducer(), leaving graphiql.tsx at only 800 LOC!

@acao
Copy link
Member

acao commented Apr 22, 2022

monaco itself will also incur a ton of breaking changes implicitly, such as entirely different keybindings, entirely different customTheme prop values, etc, so it might warrant a 2.0 itself, and the redesign a 3.0

@thomasheyenbrock
Copy link
Collaborator

Just to clarify @n1ru4l @acao: Would you release the redesign with a new major version even when there wouldn't be changes to the API (exported methods, component props, etc.)? Or do you just think it's not possible to avoid breaking changes there given the new design?

@acao
Copy link
Member

acao commented Apr 22, 2022

@thomasheyenbrock sorry, to clarify, adding monaco-graphql would necessitate breaking changes to the GraphiQL props API. as nice as it would be to just be able to swap out the underlying implementations, we have props that map very specifically to codemirror settings, and also tons of keybindings that users use every day that would just stop working, so it would feel like a major breaking change to users

monaco may also be incur some breaking changes to configuration setup, especially for CDN users

@acao
Copy link
Member

acao commented Apr 22, 2022

@thomasheyenbrock I don't think they need to go hand in hand. one can happen without the other, in whichever order. I indicated differently to @timsuchanek at one point but I've changed my mind after spending some more time on it again.

the WIP that comes second will need to rebase for the upstream, but it will be worth seperating the efforts

the editor components have almost no associated styling in either case, so it should be pretty straightforward in that regard

@thomasheyenbrock
Copy link
Collaborator

I agree, Monaco is for sure a breaking change. I agree, introducing Monaco and reworking the UI are two separate tasks that don't have to be done in one release 👍 Also, I don't have anything against releasing the redesign as v3 since the way we'll change the UI can be considered breaking even though there might not be actual API changes if we do Monaco in a separate release.

@acao
Copy link
Member

acao commented Apr 22, 2022

yes, the only API changes caused by the monaco release will be to the editorTheme prop, and a few other GraphiQL props. very minor breaking changes. from a release-notes-as PR/FAQ method, we can propose the monaco release will work like this:

congrats! there are only a few breaking changes in this release, such as allowed values for editorTheme prop. the greatest breaking change will involve learning the new keybindings. here is a document that explains the new monaco keybindings. if you're used to vscode, you'll find a lot of familiarity :)

@patrick91
Copy link
Member

@timsuchanek the calendar link doesn't seem to work for me :)

@timsuchanek
Copy link
Member Author

Thanks, everyone for bringing in your perspective!
It seems like we agree on the breakdown of tasks and have different ideas for the potential order.
I'm curious to hear, what makes you think that the design is a breaking change @n1ru4l @acao?

The reason I added that to the front, was, that for GraphiQL@1 users I want us to provide value as quickly as possible.

I'm generally definitely open to doing a major release for the design as well, just want to understand what you see breaking in there.

We seem to have a consensus, that Monaco is a breaking change, due to the reasons @acao alluded to.

I also think that the people working on this can see which order fits best. What matters is that we just keep the breakdown in releases, which we already agree on, which is great.

@timsuchanek
Copy link
Member Author

@patrick91 just added you to the event!

@timsuchanek
Copy link
Member Author

Quick update: We aligned, that the new design and the introduction of Monaco are both breaking changes.

We will therefore first go for a GraphiQL v2, which will have modernized React components together with the new design.
Then as a next step, we'll release a GraphiQL v3, which includes Monaco. Note, that in addition to the reasons that @acao mentioned for Monaco being breaking, the introduction of the Monaco webworker will be non-trivial. There are a few considerations, for example shipping a CDN-based worker, including the worker in the main process etc, that we need to figure out there.

The next step in this path is getting #2370 merged. From there on we'll continue doing steps 1) and 2) in parallel until we are at a state that we can release a release candidate for v2.

@jonathanawesome
Copy link
Collaborator

I've just started digging into the work that @thomasheyenbrock has been doing to move components into @graphiql/react, as Step 1 above, and the breaking change concerns from @n1ru4l have become clearer to me.

Leaving aside the differences in the props/API surface for now, which I haven't fully explored, I think most of the breaking UI changes have to do with the children pattern that currently looks like this:

<GraphiQL.Logo>: Replace the GraphiQL logo with your own.

<GraphiQL.Toolbar>: Add a custom toolbar above GraphiQL. If not provided, a default toolbar may contain common operations. Pass the empty <GraphiQL.Toolbar /> if an empty toolbar is desired.

<GraphiQL.Button>: Add a button to the toolbar above GraphiQL.

<GraphiQL.Menu>: Add a dropdown menu to the toolbar above GraphiQL.

<GraphiQL.MenuItem>: Items for a menu.
<GraphiQL.Select>: Add a select list to the toolbar above GraphiQL.

<GraphiQL.SelectOption>: Options for a select list.
<GraphiQL.Group>: Add a group of associated controls to the toolbar above GraphiQL. Expects children to be <GraphiQL.Button>, <GraphiQL.Menu>, or <GraphiQL.Select>.

<GraphiQL.Footer>: Add a custom footer below GraphiQL Results.

Logo

The "logo" has been relocated to an area that, potentially, could have an impact on the layout. For example,Yoga adds an icon and extra copy to the default "GraphiQL" copy.

Screen Shot 2022-07-29 at 6 04 12 PM

Screen Shot 2022-07-29 at 6 10 07 PM

I imagine there are other implementations that do something similar, so we'll need to either constrain that prop in 2.0 or offer an alternative branding option. Either way, it's currently accepting arbitrary content and seems like a breaking change to me.

Toolbar / Menu / MenuItem / Select / SelectOption / Group

This is the big one...the toolbar of old does not exist in the new design. Instead, it's functionality is split between the vertically-arranged actions within the operations editor (run operation, prettify, copy, etc) and the sidebar/plugin navigation (history, explorer, etc).

Yoga:
Screen Shot 2022-07-29 at 6 04 59 PM

Github's implementation which they refer to as "Explorer" and uses the OneGraph Explorer:
Screen Shot 2022-07-29 at 5 49 15 PM

This is easily a breaking change, and becomes gnarlier when you consider how many implementations are using OneGraph's Explorer. We can surface a new API for Operation Actions, but I don't know if it's realistic to ship a v2 without a plan for addressing OneGraph Explorer usage. This would leave a lot of implementations just plain stuck, as their primary interface is OneGraph's Explorer.

Footer

This is a weird one. I don't know why it's called "Footer", must be a legacy thing, and the only implementation I'm aware of is @benjie's postgraphiql (I'm sure there are others...if you're reading, please comment!).

We do have some designs that might map to the current architecture:
Screen Shot 2022-07-29 at 7 09 24 PM

But it's still a breaking change because the new design doesn't have a singular "footer" space for arbitrary content.

So...all of that to say that I'm convinced that the new design, as wonderful and beautiful and necessary as it is, is a breaking change to existing implementations. We have more thinking to do around what is included in a 2.0 release.

@acao
Copy link
Member

acao commented Jul 30, 2022

@jonathanawesome we had an issue pinned for years that announces the deprecation of these interfaces for 2.x, it’s still in the readme i think

Either way it would be good to provide a migration path! These are widely used and the baseline of 2.0.0 we promised is that each oss framework could do some basic branding customizations at least - color theme, maybe typefaces, and logo at least.

Perhaps the other regions aren’t as important now that we’re shipping the editor with new ways to customize. As long as we can allow them to add their own sidebar components that use the hooks, and be able to add commands, they should have most of the flexibility they need.

If they need a more customized layout, they can “eject” and copy over the few components that make up graphiql and compose their own editor

@benjie
Copy link
Member

benjie commented Jul 30, 2022

For context, PostGraphiQL's footer serves two purposes:

  1. a single line detailing helpful links to the docs, examples, support, and of course sponsorship ;)
  2. a large expansion containing the SQL that was executed from the GraphQL query/the SQL query plan/etc

Point 1 would be nice to have somewhere. Point 2 would be much better served by the alternative views that the new design allows for however it is pretty important (to me at least) to be able to see the query, the result, AND the SQL queries all at the same time. So I wouldn't like to just replace the result with the SQL queries/etc, that would make it concretely less useful to me.

Basically what I'm saying is that point 2 shouldn't be in the footer, it's only there because there was nowhere else suitable that we could put it in the older GraphiQL. Plugins should help.

Point 1 could easily go under the entire GraphiQL interface as a separate component I suppose; much like the footer on a website. It may not need to be factored into the 2.x design.

@thomasheyenbrock
Copy link
Collaborator

I know that the child pattern has been announced to be dropped in v2, but I'm not sure we should as it's widely used.

  • The logo can still be customized using GraphiQL.Logo, this should allow the GraphQL Yoga use-case where a custom icon is added next to the "GraphiQL" text.

  • The toolbar can be customized in two ways:

    • Additional content can be added using the toolbar prop, leaving the existing buttons (prettify, copy, and merging fragments) in place
    • The default toolbar can be replaced by passing GraphiQL.Toolbar as child.

    The former would allow GraphQL Yoga and PostGraphile to add buttons to the toolbar.

  • Regarding the footer, in the current state of our v2 efforts it still exists. In my opinion the "response tools" we have in the design are too much for v2. They are basically part of a fully thought through plugin API, and I don't think this is feasible in v2. We wanted to ship as quickly as possible, that's why I think v2 should only take care of introducing the new design, excluding the doc explorer and the complete plugin structure.

@acao
Copy link
Member

acao commented Jul 30, 2022

but I'm not sure we should as it's widely used

excellent point, and you are so correct!

and it's not impossible to implement or type either! I'm sure you've found some ways to improve it @thomasheyenbrock I haven't even looked. it's not even an uncommon pattern. And it's convenient. And with the hooks, we can do so much more than before! Toolbar buttons and Footer will will be even more powerful than before with @graphiql/react!

Everyone is making great points. Should this move to a discussion about the future of the child prop pattern? We can copy and paste some of these comments to start their own threads honestly! haha

@jonathanawesome
Copy link
Collaborator

I'm not a huge fan of the children pattern going forward, as the new design allows for more customization and I think exposing these options as typed props is a tighter experience. For those users that find a typed interface too tight, they'd likely be comfortable plucking the components out of @graphiql/react and building something of their own.

  • The logo can still be customized using GraphiQL.Logo, this should allow the GraphQL Yoga use-case where a custom icon is added next to the "GraphiQL" text.

I'm suggesting that allowing arbitrary content, which is what GraphiQL.Logo is, is not the correct solution considering the new design. Not only is horizontal space in that Tab row at a premium, but it doesn't seem like the right place for a mark/lockup/logo.

I'd like to see the GraphiQL type always be visible in that spot and serve as a link to the repo/documentation.

Any sort of branding and/or arbitrary content, a la @benjie's use case, might be better served by using a Pane Plugin. This doesn't need to be available via the full-blown plugin interface, we could offer a much simpler interface for v2 that allows for something like this:

Screen Shot 2022-07-30 at 9 36 10 AM

👆This is also a great solution for removing the wall of welcome/how-to text that's currently in the operations editor on initial load.
Screen Shot 2022-07-30 at 9 44 39 AM

  • The toolbar can be customized in two ways:

    • Additional content can be added using the toolbar prop, leaving the existing buttons (prettify, copy, and merging fragments) in place
    • The default toolbar can be replaced by passing GraphiQL.Toolbar as child.

    The former would allow GraphQL Yoga and PostGraphile to add buttons to the toolbar.

The toolbar has been re-contextualized, so any buttons that are being added (by either of the two methods) that aren't operation actions will be very much out of place. Here's what PostGraphiQl does.

  • Regarding the footer, in the current state of our v2 efforts it still exists. In my opinion the "response tools" we have in the design are too much for v2. They are basically part of a fully thought through plugin API, and I don't think this is feasible in v2. We wanted to ship as quickly as possible, that's why I think v2 should only take care of introducing the new design, excluding the doc explorer and the complete plugin structure.

Totally agree re: response tools and shipping a well-formed plugin structure for v2. My concern with GraphiQL.Footer being used as-is in v2 is that if it doesn't work for some implementations (it's arbitrary content, who knows how many variations exist) we'll end up scrambling for a solution just to get those folks into v2.

@thomasheyenbrock
Copy link
Collaborator

@jonathanawesome great points you bring up here!

Regarding the logo: I like the idea of putting it to the top left, but there are other subtleties to consider here like you already mentioned, like reusing the plugin pane on the left to have a dedicated view that shows custom content. Instead of figuring this out for v2, I'd propose to keep the current way where a user can switch out the "logo" next to the tabs on the top right as it's a very flexible solution. (Yes we do allow arbitrary content, but we can't protect GraphiQL users from messing up the design in all cases anyways.)

Just to clarify around the footer: Implementations that use GraphiQL.Footer today should not need to change anything about it in order to upgrade to v2 since the placement and the layout is currently still the same.

I like the idea of replacing the child pattern with typed props eventually, again I just think that's too much for v2. I believe we have enough breaking changes for that major version, and adding more will just make it harder for folks to upgrade. It might also be possible to introduce these props in future minors, officially deprecate the child pattern and then remove it in v3 or a future major update.

TL;DR I see most of this out of scope for v2, but would love for us to further discuss it with more focus by opening dedicated issues.

@SimenB
Copy link
Contributor

SimenB commented Sep 3, 2022

Now v2 is out (:tada:) should this be closed and/or unpinned?

@Sytten
Copy link

Sytten commented Sep 3, 2022

I am also wondering if the refactor of the various components is done since the underlining issues have not been closed. We are looking to build another implementation for vue people and we would like to know of we can use a couple building blocks that would already be present.

@thomasheyenbrock
Copy link
Collaborator

Thanks for the callouts @SimenB and @Sytten! We should definitely do some housekeeping, close out some historic issues that have been resolved with v2 and consolidate the various roadmap-issues. I'll try to spend some time over the coming days to clean this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants