thecodingmachine / graphqlite-bundle

A Symfony bundle for thecodingmachine/graphqlite.
35 stars 40 forks source link

Remove overblog/graphiql-bundle #206

Open fogrye opened 7 months ago

fogrye commented 7 months ago

This package struggles hard with migration to Symfony v7 plus version of graphiql it brings is old by default and we don't have any info in docs how to update that. I would copy approach of how ApiPlatform does this thing with using 1 route, twig and minified js with latest version of graphiql. I would deliver this with https://github.com/thecodingmachine/graphqlite-bundle/pull/203 because now, at least in my case, overblog/graphiql-bundle is main blocker of migration to Symfony v7.

aszenz commented 6 months ago

This package struggles hard with migration to Symfony v7 plus version of graphiql it brings is old by default and we don't have any info in docs how to update that. I would copy approach of how ApiPlatform does this thing with using 1 route, twig and minified js with latest version of graphiql. I would deliver this with #203 because now, at least in my case, overblog/graphiql-bundle is main blocker of migration to Symfony v7.

We could also just split/remove the graphiql route into it's own bundle since it's not really this bundle's job to provide a graphql editor, this bundle is about symfony integration

fogrye commented 6 months ago

@aszenz IMHO it's nice for newbie but I would not advocate it that much. Me personally don't have access to create repos under thecodingmachine but I think it would be nice to have separate repo as a suggestion if you actually need it. If we can somehow make a new repo it would be great.

SCIF commented 6 months ago

@fogrye ,

remove the graphiql route into it's own bundle since it's not really this bundle's job to provide a graphql editor

Totally «for» this statement. A nice tooling is not something bundle should be really worry about. Moreover, overblog's bundle is not prod-required package so having it as a dep is actually a bad idea.

fogrye commented 6 months ago

As this issue takes more attention recently I encourage anyone to make a PR for it. Unfortunately I have too much on myself so I won't be able to contribute to it in next couple of weeks.

homersimpsons commented 6 months ago

To share my opinion on this: I'm not against removing it, specifically if it blocks some upgrade. But I know this is a valuable tool for devlopment and having it integrated certainly improves the developer experience.

If we remove it, there should be an easy / documented way to have such tool, I know Postman or similar applications provides this so maybe we can just redirect the users to those tools.

goffyara commented 5 months ago

My opinion is that it's better to remove the dependency on the bundle. Anyone who needs it will be able to configure it themselves or use another documentation tool of their choice. Moreover, the native ability to override the endpoint to which the overblog bundle sends requests is currently blocked, and it's necessary to write a separate CompilerPass just for this.

fogrye commented 3 months ago

I have created wiki page, I believe it is enough to remove the dependency.

homersimpsons commented 3 months ago

I have created wiki page, I believe it is enough to remove the dependency.

Thank you! The wiki is probably not that easy to discover, but I would expect people using GraphQL to search for such kind of frontend (GraphiQL, Postman, hoppscotch, etc...)

dyonis commented 3 months ago

I have created wiki page, I believe it is enough to remove the dependency.

I tried this extension and realized that it is not convenient and slows down the computer a lot. It was very easy to create a simple controller and copy the twig template to set up the playground:

class GraphiqlController extends AbstractController
{
    #[Route('/api')]
    public function index(): Response
    {
        return $this->render('GraphiQL/index.html.twig', [
            'endpoint' => '/graphql',
        ]);
    }
}
<!DOCTYPE html>
<html lang="en">
<head>
    {% block head %}
    <meta charset=utf-8/>
    <meta name="viewport" content="user-scalable=no, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, minimal-ui">
    <title>{% block title %}GraphQL Playground{% endblock title %}</title>
    <link rel="shortcut icon" href="//cdn.jsdelivr.net/npm/graphql-playground-react/build/favicon.png" />
    {% block style %}
    <link rel="stylesheet" href="//cdn.jsdelivr.net/npm/graphql-playground-react/build/static/css/index.css?" />
    {% endblock style %}
    {% block script %}
    <script src="//cdn.jsdelivr.net/npm/graphql-playground-react/build/static/js/middleware.js?"></script>
    {% endblock script %}
    {% endblock head %}
</head>

<body>
{% block body %}
<div id="root">
    {% block body_style %}
    <style>
        body {
            background-color: rgb(23, 42, 58);
            font-family: Open Sans, sans-serif;
            height: 90vh;
        }

        #root {
            height: 100%;
            width: 100%;
            display: flex;
            align-items: center;
            justify-content: center;
        }

        .loading {
            font-size: 32px;
            font-weight: 200;
            color: rgba(255, 255, 255, .6);
            margin-left: 20px;
        }

        img {
            width: 78px;
            height: 78px;
        }

        .title {
            font-weight: 400;
        }
    </style>
    {% endblock body_style %}
    <img src='//cdn.jsdelivr.net/npm/graphql-playground-react/build/logo.png' alt=''>
    <div class="loading"> Loading
        <span class="title">GraphQL Playground</span>
    </div>
</div>
{% block body_script %}
<script>
    window.addEventListener('load', function () {
        GraphQLPlayground.init(document.getElementById('root'), {
            endpoint: {{ endpoint | json_encode | raw }}
        })
    })
</script>
{% endblock body_script %}
{% endblock body %}
</body>
</html>
homersimpsons commented 3 months ago

@dyonis Do you suggest we add this script in the bundle in place of overblog/graphiql-bundle?

I think we can even go with just https://github.com/graphql/graphiql/blob/main/examples/graphiql-cdn/index.html

dyonis commented 3 months ago

@dyonis Do you suggest we add this script in the bundle in place of overblog/graphiql-bundle?

yes, why not? I wouldn't want such a small thing to block this bundle from being updated, because I use it in several projects and can't upgrade to symfony 7

enricobono commented 4 hours ago

Hello @homersimpsons and @fogrye. @mcg-web just merged https://github.com/overblog/GraphiQLBundle/pull/40 that adds support to Symfony 7. They should release a new version of overblog/GraphiQLBundle soon:

If the purpose of the current PR was to allowgraphqlite-bundle to be compatible with Symfony 7, we may not need it anymore.