sveltekit support #229

PabloSzx · 2021-08-24T20:17:56Z

closes #146

@benbender if you want to collaborate on this you are more than welcome 👍

vercel · 2021-08-24T20:17:58Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/theguild/graphql-ez/8GCgzXxpdUKdcYfu42C72kBCrxke
✅ Preview: https://graphql-ez-git-sveltekit-theguild.vercel.app

PabloSzx · 2021-08-24T21:47:52Z

@benbender Here is the first working version https://github.com/PabloSzx/graphql-ez/blob/sveltekit/examples/sveltekit/src/routes/graphql.ts if you want please take a look and test it and give opinions or suggestions https://github.com/PabloSzx/graphql-ez/blob/sveltekit/packages/sveltekit/main/src/index.ts

What I did to reduce the friction with the current node.js integrations is to create a fake IncomingMessage that only has access to "headers" and "method", which is what most of the integrations or envelop plugins use anyways

benbender · 2021-08-25T07:38:10Z

Hey! Thanks for the effort! I would love to play with it and so I'll just added an additional route with the newly created graphq-ez-handler into my play-project and it seems to work fine initially with a mildly complex schema generated by giraphql. Great job!

Problem atm seems to be that the sveltekit-integration isn't known to the core yet and therefor I can't test any plugins without patching the sveltekit-integration into the compatibility-list. Is there some kind of undocumented switch to turn off the compatibility check or something? And if not, might it be worth adding one for development?

PabloSzx · 2021-08-25T07:46:13Z

Hey! Thanks for the effort! I would love to play with it and so I'll just added an additional route with the newly created graphq-ez-handler into my play-project and it seems to work fine initially with a mildly complex schema generated by giraphql. Great job!

Problem atm seems to be that the sveltekit-integration isn't known to the core yet and therefor I can't test any plugins without patching the sveltekit-integration into the compatibility-list. Is there some kind of undocumented switch to turn off the compatibility check or something? And if not, might it be worth adding one for development?

the compatibility list doesn't make sense to turn off for the specific plugins that requires it because the "onIntegrationRegister" lifecycle has to be hardcoded for the integration that it supports 😞

PabloSzx · 2021-08-25T07:49:39Z

for the IDE plugins like graphiql and altair or voyager, the plugins will need to export an extra handler that adds support for sveltekit, it's not hard just reusing the existing handlers but adapted for the sveltekit format, but the altair requires a dynamic route

for the plugins:

upload
websockets
new sse

they are virtually impossible to add support for sveltekit 😞

benbender · 2021-08-25T07:51:12Z

What I had in mind was a simple option, probably undocumented, which just circumvents the checks entirely. Not meant to be used regularly, but just for development of new plugins/integrations.

So for example one could have something like:

    ezApp = createEZAppFactory(
      {
        integrationName: "sveltekit",
        disableCompatibilityChecks: true
      },
      appConfig
    );

And a check for it here:

graphql-ez/packages/core/main/src/app.ts

Line 60 in 9878698

if (compatibilityList && !compatibilityList.includes(integrationName)) {

PabloSzx · 2021-08-25T07:54:47Z

What I had in mind was a simple option, probably undocumented, which just circumvents the checks entirely. Not meant to be used regularly, but just for development of new plugins/integrations.

So for example one could have something like:
    ezApp = createEZAppFactory(
      {
        integrationName: "sveltekit",
        disableCompatibilityChecks: true
      },
      appConfig
    );
And a check for it here:

graphql-ez/packages/core/main/src/app.ts

Line 60 in 9878698

if (compatibilityList && !compatibilityList.includes(integrationName)) {

the compatibility list is only there for the lifecycle "onIntegrationRegister", the plugins that don't use that lifecycle, don't specify it, there isn't any purpose behind disabling them, and if you are creating new plugins, you can simply don't specify the compatibilityList field / not use onIntegrationRegister / add the integration you are working on to the list

the onIntegrationRegister uses integration specific context, that won't be available for the logic it uses, that is why the check is there

benbender · 2021-08-25T08:44:43Z

Thanks for the invite! I'll have a look onto altair/graphiql later today or tomorrow as I have some things to check on my todo-list beforehand...

There are also some slight issues with the typings of the new sveltekit-integration as, f.e., I'm using a global type for Locals which isn't correctly picked up by the Request/ServerRequest-type. I just noticed this while setting up my context...

For everything related to upload/multipart/streams in sveltekit, there might be some hope for the future as the Whatwg Streams-API gets more support (deno, node16, ...) and there are some quite active discussions about it at sveltejs/kit#1563, sveltejs/kit#2212 etc and there might also be a chance for some sort of support for connect-handlers before 1.0.

benbender · 2021-08-25T13:38:15Z

I've added the most primitive support for graphiql here.

Before going further, I wanted to ask, if there is any specific reason why you've split the routes for the nextjs-handler between graphql and graphiql?

The straight-forward way to implement compatibility for sveltekit, in the same manner like you did for nextjs, seems to be to duplicate the GraphiQLHandler-method and adapt it for sveltekit.

That approach seems repetitive to me and I would have opted for another approach to handle the split between the graphiql/graphql-handlers directly inside the graphql-ez-app - much like graphql-helix did with its shouldRenderGraphiQL()-method. Following that approach would enable the nextjs/sveltekit-integrations also to be simply added via the plugin-interface without having the need to add another route-handler...

PabloSzx · 2021-08-25T14:06:19Z

Before going further, I wanted to ask, if there is any specific reason why you've split the routes for the nextjs-handler between graphql and graphiql?

I don't think there is a specific reason for it, it was just more of an opinionated decision, I felt like going to graphiql should be a different path and 100% conscious decision, just like going to altair is, but I understand 100% that having it in the same path with logic like "if (shouldRenderGraphiQL(" is something a lot of people expect, I will work on it 👍 it might mean some changes for some integrations

PabloSzx · 2021-08-25T16:51:23Z

Before going further, I wanted to ask, if there is any specific reason why you've split the routes for the nextjs-handler between graphql and graphiql?

I don't think there is a specific reason for it, it was just more of an opinionated decision, I felt like going to graphiql should be a different path and 100% conscious decision, just like going to altair is, but I understand 100% that having it in the same path with logic like "if (shouldRenderGraphiQL(" is something a lot of people expect, I will work on it 👍 it might mean some changes for some integrations

#236 🎉 I want to update the graphiql instance to have the changes of contra/graphql-helix#44 & contra/graphql-helix#37 before merging tho 👌

PabloSzx · 2021-08-25T19:20:39Z

Now by default, the graphiql plugin points to the same path as the API, just like vanilla graphql-helix and apollo server does it👌

https://github.com/PabloSzx/graphql-ez/releases/tag/@graphql-ez/[email protected]

PabloSzx · 2021-08-29T01:16:23Z

There are also some slight issues with the typings of the new sveltekit-integration as, f.e., I'm using a global type for Locals which isn't correctly picked up by the Request/ServerRequest-type. I just noticed this while setting up my context...

@benbender what do you think is the solution for that, you are using those custom types in the "buildContext"?

I came up with this solution, a custom type that allows you to specify a custom locals

changeset-bot · 2021-08-29T17:39:11Z

🦋 Changeset detected

Latest commit: 3670f22

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 31 packages

Name	Type
@graphql-ez/plugin-altair	Minor
graphql-ez	Patch
@pablosz/graphql-helix-graphiql	Patch
@graphql-ez/plugin-graphiql	Minor
@graphql-ez/plugin-voyager	Minor
@graphql-ez/sveltekit	Patch
@graphql-ez/cloudflare	Patch
@graphql-ez/express	Patch
@graphql-ez/express-testing	Patch
@graphql-ez/fastify	Patch
@graphql-ez/fastify-testing	Patch
@graphql-ez/hapi	Patch
@graphql-ez/hapi-testing	Patch
@graphql-ez/http	Patch
@graphql-ez/http-testing	Patch
@graphql-ez/koa	Patch
@graphql-ez/koa-testing	Patch
@graphql-ez/nextjs	Patch
@graphql-ez/nextjs-testing	Patch
@graphql-ez/plugin-automatic-persisted-queries	Patch
@graphql-ez/plugin-codegen	Patch
@graphql-ez/plugin-dataloader	Patch
@graphql-ez/plugin-modules	Patch
@graphql-ez/plugin-scalars	Patch
@graphql-ez/plugin-schema	Patch
@graphql-ez/plugin-sse	Patch
@graphql-ez/plugin-upload	Patch
@graphql-ez/plugin-websockets	Patch
@graphql-yoga/express	Patch
graphql-yoga	Patch
@graphql-yoga/preset	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel bot deployed to Preview August 24, 2021 21:45 View deployment

PabloSzx force-pushed the sveltekit branch from 2950d99 to ca28d19 Compare August 24, 2021 23:06

vercel bot deployed to Preview August 24, 2021 23:06 View deployment

vercel bot deployed to Preview August 25, 2021 07:37 View deployment

PabloSzx force-pushed the sveltekit branch from 2c8981c to 724f092 Compare August 25, 2021 07:40

vercel bot deployed to Preview August 25, 2021 07:44 View deployment

PabloSzx force-pushed the main branch from 7acf58b to cb57cb6 Compare August 26, 2021 18:48

PabloSzx force-pushed the sveltekit branch from 724f092 to 22acad5 Compare August 26, 2021 22:09

vercel bot deployed to Preview August 26, 2021 22:09 View deployment

PabloSzx force-pushed the sveltekit branch from 22acad5 to 3aff4d6 Compare August 28, 2021 23:42

vercel bot deployed to Preview August 28, 2021 23:43 View deployment

vercel bot deployed to Preview August 29, 2021 02:15 View deployment

PabloSzx force-pushed the sveltekit branch from a5d799f to 67672c5 Compare August 29, 2021 02:40

vercel bot deployed to Preview August 29, 2021 02:41 View deployment

PabloSzx force-pushed the sveltekit branch from 67672c5 to ed1f053 Compare August 29, 2021 15:00

vercel bot deployed to Preview August 29, 2021 15:00 View deployment

vercel bot deployed to Preview August 29, 2021 16:56 View deployment

vercel bot deployed to Preview August 29, 2021 17:26 View deployment

PabloSzx force-pushed the sveltekit branch from 1237121 to 918acc0 Compare August 29, 2021 17:39

vercel bot deployed to Preview August 29, 2021 17:39 View deployment

vercel bot deployed to Preview August 30, 2021 00:16 View deployment

vercel bot deployed to Preview August 30, 2021 01:47 View deployment

vercel bot deployed to Preview August 30, 2021 02:37 View deployment

PabloSzx added 20 commits August 30, 2021 00:39

add sveltekit example

d0633f5

create @graphql-ez/sveltekit

d49c898

update

b413291

sveltekit integration progress

4985b81

graphiql support new SvelteKit integration

173cf8c

fix detect share graphiql url

c6448ae

fix graphiql http pathname compare

37b97d0

sveltekit respect manual path

2c3cda5

altair progress

7cbc455

altair add support for SvelteKit

f1d400d

simplify Altair Render Logic

341c0ce

update snapshots

6d299d3

fix altair render options

49be89d

voyager Add support for SvelteKit

0cb6c97

format

d822050

fix altair sveltekit content-type

9796ed9

not optional sveltekit prop in build context args

12420cc

add sveltekit docs

b1b43ce

minor improvement

1798489

add sveltekit tests

3670f22

PabloSzx force-pushed the sveltekit branch from faa80bb to 3670f22 Compare August 30, 2021 04:40

vercel bot deployed to Preview August 30, 2021 04:40 View deployment

PabloSzx merged commit 7828f5e into main Aug 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sveltekit support #229

sveltekit support #229

PabloSzx commented Aug 24, 2021 •

edited

Loading

vercel bot commented Aug 24, 2021 •

edited

Loading

PabloSzx commented Aug 24, 2021

benbender commented Aug 25, 2021

PabloSzx commented Aug 25, 2021

PabloSzx commented Aug 25, 2021 •

edited

Loading

benbender commented Aug 25, 2021

PabloSzx commented Aug 25, 2021 •

edited

Loading

benbender commented Aug 25, 2021

benbender commented Aug 25, 2021

PabloSzx commented Aug 25, 2021

PabloSzx commented Aug 25, 2021

PabloSzx commented Aug 25, 2021 •

edited

Loading

PabloSzx commented Aug 29, 2021 •

edited

Loading

changeset-bot bot commented Aug 29, 2021 •

edited

Loading

sveltekit support #229

sveltekit support #229

Conversation

PabloSzx commented Aug 24, 2021 • edited Loading

vercel bot commented Aug 24, 2021 • edited Loading

PabloSzx commented Aug 24, 2021

benbender commented Aug 25, 2021

PabloSzx commented Aug 25, 2021

PabloSzx commented Aug 25, 2021 • edited Loading

benbender commented Aug 25, 2021

PabloSzx commented Aug 25, 2021 • edited Loading

benbender commented Aug 25, 2021

benbender commented Aug 25, 2021

PabloSzx commented Aug 25, 2021

PabloSzx commented Aug 25, 2021

PabloSzx commented Aug 25, 2021 • edited Loading

PabloSzx commented Aug 29, 2021 • edited Loading

changeset-bot bot commented Aug 29, 2021 • edited Loading

🦋 Changeset detected

PabloSzx commented Aug 24, 2021 •

edited

Loading

vercel bot commented Aug 24, 2021 •

edited

Loading

PabloSzx commented Aug 25, 2021 •

edited

Loading

PabloSzx commented Aug 25, 2021 •

edited

Loading

PabloSzx commented Aug 25, 2021 •

edited

Loading

PabloSzx commented Aug 29, 2021 •

edited

Loading

changeset-bot bot commented Aug 29, 2021 •

edited

Loading