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

Replace API Playgrounds with GraphiQL #3246

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

Conversation

toBeOfUse
Copy link
Contributor

@toBeOfUse toBeOfUse commented Nov 30, 2024

Description

The GraphQL playground that Vendure has been using is an abandoned project; Apollo warns us when running npm install that it is no longer actively supported, and it has UI issues in modern browsers where tooltips never disappear (#3244). Therefore, it was proposed to replace it with GraphiQL, which is a modern equivalent of this playground that does not have its problems.

It was proposed to do this by bundling GraphiQL and serving it from an Apollo plugin that implements renderLandingPage. Sadly, this function can literally only return a single HTML string; the Apollo documentation says that this is an intentional limitation, and that if it annoys you, you should implement something in your web framework to serve a playground (so, in this case, a NestJS controller, presumably.) So, this PR adds an Apollo plugin that serves GraphiQL as a single 1.5mb HTML string.

I am not 1000% confident that this is the way to go, this time. For one thing, bundling a package like GraphiQL into a single HTML string is unusual. For another, this adds some quite heavy dependencies to the @vendure/core package: GraphiQL depends on react and react-dom, and then you need some kind of bundler to produce the final result. I added Webpack and some loaders for this purpose, since Webpack is already in use for the Admin UI plugin. Then, I committed the bundled GraphiQL file to this branch, so that it wouldn't have to be re-built by everyone who installed @vendure/core for the first time (so the aforementioned dependencies end up being dev dependencies only.)

So, my questions are: Would it be better to implement this as a NestJS controller, and serve a directory of files for GraphiQL instead of a string? And, would it be better to implement GraphiQL as, perhaps, a separate package, instead of adding React and Webpack to the core, which previously was largely backend code?

I welcome feedback on this PR. But it does solve the problem.

Breaking changes

This changes the pages that show up when you go to /admin-api or /shop-api with the playgrounds enabled in vendure-config.ts. That said, this shouldn't break any code or disrupt anyone's workflow, since GraphiQL provides the same basic features.

Except, I didn't implement the playground configuration options, which could be used to configure the Apollo playground:

export interface RenderPageOptions extends MiddlewareOptions {
  version?: string
  cdnUrl?: string
  env?: any
  title?: string
  faviconUrl?: string | null
}

You can see the other settings here. Some of them would be very difficult to try to emulate with GraphiQL: for example, the Apollo playground offered very detailed customization of colors through the EditorColours interface, but GraphiQL just allows for colors to be changed with predefined CodeMirror themes or CSS variables. Some decision needs to be made regarding how much backward-compatibility to provide.

Also, making this change would perhaps imply that the screenshots of the old playground that exist in the docs should be updated. I could do this; there don't seem to be a ton.

Screenshots

This was checked using the included dev-server package.

image

When not signed into the Admin API:

image

After signing in, via the Admin UI at /admin:

image

When the playground is turned off by setting adminApiPlayground (or shopApiPlayground) to false in the Vendure config:

image

Checklist

📌 Always:

  • I have set a clear title
  • My PR is small and contains a single feature
  • I have checked my own PR

👍 Most of the time:

  • I have added or updated test cases
  • I have updated the README if needed

Copy link

vercel bot commented Nov 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview Nov 30, 2024 8:09am

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
28 Security Hotspots
E Reliability Rating on New Code (required ≥ A)
E Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@toBeOfUse toBeOfUse changed the base branch from master to minor November 30, 2024 08:29
@toBeOfUse toBeOfUse changed the base branch from minor to master November 30, 2024 08:30
@toBeOfUse
Copy link
Contributor Author

SonarCloud is very mad at the Webpack-generated bundle code.

Also, let me know if I should rebase this onto the minor branch.

@michaelbromley
Copy link
Member

Wow great! Thanks for taking the time to investigate. I don't mind the react dev dependencies because we already have those as in the admin ui package. I'm wondering whether we can use vite or rollup for the bundling, since we already use them elsewhere for bundling the UI devkit.

Yes, this should be made against the minor branch.

And I have a slight concern that the was it resolves the html string might be brittle in situations where the server code is being itself bundled, which happens when using Nx for example.

It might be a good idea to initially make this plugin opt-in, eg via a playground: 'graphiql' option.

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.

2 participants