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.

Sign up for GitHub

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

Jump to bottom

Remove overblog/graphiql-bundle #206

Closed
fogrye opened this issue Apr 13, 2024 · 13 comments
Closed

Remove overblog/graphiql-bundle #206

fogrye opened this issue Apr 13, 2024 · 13 comments

Comments

@fogrye
Copy link
Contributor

fogrye commented Apr 13, 2024

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.
@aszenz
Copy link
Contributor

aszenz commented May 2, 2024

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
Copy link
Contributor Author

fogrye commented May 2, 2024

@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.

@fogrye fogrye mentioned this issue May 2, 2024
Closed
@SCIF
Copy link

SCIF commented May 3, 2024

@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
Copy link
Contributor Author

fogrye commented May 3, 2024

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
Copy link
Collaborator

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
Copy link

goffyara commented Jun 3, 2024

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.

@homersimpsons homersimpsons changed the title Replace overblog/graphiql-bundle with something else Remove overblog/graphiql-bundle Jun 8, 2024
@fogrye
Copy link
Contributor Author

fogrye commented Jul 29, 2024

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

@homersimpsons
Copy link
Collaborator

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
Copy link

dyonis commented Aug 2, 2024

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
Copy link
Collaborator

@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
Copy link

dyonis commented Aug 4, 2024

@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 enricobono mentioned this issue Nov 15, 2024
Merged
@enricobono
Copy link
Collaborator

Hello @homersimpsons and @fogrye.
@mcg-web just merged overblog/GraphiQLBundle#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 allow graphqlite-bundle to be compatible with Symfony 7, we may not need it anymore.

@andrew-demb
Copy link
Collaborator

Support for overblog/graphiql-bundle v1 (compatible with Symfony 7) was added in #227

Please reopen this issue if you think this is not enough and we should make this a "soft" dependency (or drop it at all).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@SCIF @enricobono @goffyara @andrew-demb @homersimpsons @aszenz @dyonis @fogrye