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

Use server's own graphiql package instead of from CDN #295

Closed
ephemer opened this issue Jan 31, 2017 · 22 comments
Closed

Use server's own graphiql package instead of from CDN #295

ephemer opened this issue Jan 31, 2017 · 22 comments

Comments

@ephemer
Copy link
Contributor

ephemer commented Jan 31, 2017

graphql-server currently manages its own version of GraphiQL in package-internal code. Maybe I'm missing something but this seems unnecessary given we have npm/yarn.

Is there anything stopping us from serving the version of graphiql installed on the server, instead of doing this? That way we wouldn't need to manage the version separately to what is available on npm.

I realise this would involve serving files from the node server in some way, which is an anti-pattern in production, but I wouldn't expect the proposed (or present) method to be used in production as is anyway.

I'm interested in others' thoughts about this. It seems a shame to always be at least a little bit behind the cutting edge in terms of Developer Experience just because we're using graphql-server.

@stubailo
Copy link
Contributor

Ironically, we're actually ahead of express-graphql: https://github.com/graphql/express-graphql/blob/87fc6d527d0d8fe128130a26bef39e5be2b26da5/src/renderGraphiQL.js#L19

Anyway, I agree it would be a good idea to see if we can get it from npm, but that is a bit more complex because you need a build system, etc. Perhaps we can start by making a list of what we need to do to make this happen?

@stubailo
Copy link
Contributor

Oh lol I just realized you sent that PR. But yeah, we essentially copied that logic from express-graphql.

@DxCx
Copy link
Contributor

DxCx commented Feb 1, 2017

hi @ephemer,
i don't see why this is better then just serve from CDN..?
the thing is that to make it work from a package there is much work to be done,
and i can't the find the reason to invest time into that.. can you explain us please? :D

@stubailo
Copy link
Contributor

stubailo commented Feb 1, 2017

I think the main reason is to stay up to date with the newest version of GraphiQL, and not have to wait for the CDN url to be updated. Perhaps we can have an option in renderGraphiQL for the version number so that it can be increased manually when necessary?

@DxCx
Copy link
Contributor

DxCx commented Feb 2, 2017

yes @stubailo i agree with you, we can set default in const and pass another option.
great idea! :)
@ephemer what do you think of the solution?

@ephemer
Copy link
Contributor Author

ephemer commented Feb 3, 2017

Oh yeah! That's easy and solves the problem, nice @stubailo

@DxCx
Copy link
Contributor

DxCx commented Feb 3, 2017

cool @ephemer can you please open another issue for implementing this and close this instead.
also if you want to submit the PR for the actual change, you are welcome =)

@helfer
Copy link
Contributor

helfer commented Feb 4, 2017

This would be a great project for contributor week btw 😉

@ephemer
Copy link
Contributor Author

ephemer commented Feb 13, 2017

@helfer sure, we have a deadline this week so I don't think I'll get to it but I'd be happy to give it a shot when I have a bit more time on my hands

@helfer
Copy link
Contributor

helfer commented Jun 28, 2017

I still think this would be neat to implement.

@jacobbogers
Copy link

Hi, anyone working on this already, can I help out or I will start myself

@luchillo17
Copy link

I have an issue with the CDN for react.min.js, however if i change the js string to react.js it works ok:
image

@stubailo
Copy link
Contributor

@jacobbogers or @luchillo17 please feel free to get started! I think this would be a great improvement.

@gilesbradshaw
Copy link

Definitely a good idea not only for when the cdn goes down but also for when working in remote locations without internet.

@gilesbradshaw
Copy link

this is related to #506

@stubailo
Copy link
Contributor

Does anyone want to sign up to get started on this? Shouldn't take more than a few hours to set up the build and make it run from a local setup.

I'd say a script that just downloads the CDN version and inlines it into the package, or builds with Webpack, would be perfect.

@nadavten
Copy link

any news ? common

@rbureau85
Copy link

is this a thing yet?

@stubailo
Copy link
Contributor

There's a work in progress PR here: #673 please help!

@abernix
Copy link
Member

abernix commented Sep 24, 2018

Apollo Server 2.0 ships with graphql-playground as a direct dependenct, rather than a CDN-hosted version, so I believe this issue can be closed. Happy to reopen if anyone feels different. Thanks for reporting this originally!

#1421

@penx
Copy link

penx commented Sep 26, 2018

Apollo Server 2.0 ships with graphql-playground as a direct dependenct, rather than a CDN-hosted version

Using apollo-server@2.1.0 and following the getting started guide I get a graphql-playground instance using assets from an external CDN, so I am pretty sure this issue is still open.

As above, there are more details in #1421 and I am working on a fix in #1734

@abernix
Copy link
Member

abernix commented Oct 25, 2018

I was incorrect above, this is indeed still an issue, but let's track it in #1421.. Thanks for your work in #1734 — we'll consider how to make that more permanent in Apollo Server, though I think your demonstration in #1734 (comment) is pretty close to what that will end up being (but less explicitly).

trevor-scheer pushed a commit that referenced this issue May 6, 2020
…iant

Update op-reg plugin to use graphVariant and add support for APOLLO_GRAPH_VARIANT
trevor-scheer pushed a commit that referenced this issue May 12, 2020
…iant

Update op-reg plugin to use graphVariant and add support for APOLLO_GRAPH_VARIANT
trevor-scheer pushed a commit that referenced this issue May 14, 2020
…iant

Update op-reg plugin to use graphVariant and add support for APOLLO_GRAPH_VARIANT
trevor-scheer pushed a commit that referenced this issue May 14, 2020
…iant

Update op-reg plugin to use graphVariant and add support for APOLLO_GRAPH_VARIANT
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests