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

Upgrade to PostGraphile V5 #355

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

Upgrade to PostGraphile V5 #355

wants to merge 13 commits into from

Conversation

benjie
Copy link
Member

@benjie benjie commented Mar 9, 2023

Note that since PostGraphile V5 is still pre-release sponsors-only software right now, to use this PR you will need to know the secret from https://grafast.org/caveats/ . If you don't have access and are already a sponsor, ping Jem (on the Graphile Discord, via Twitter, or via email on their first name at graphile.com) and they'll help you get set up.

IMPORTANT: when viewing the diff, you should hide whitespace changes otherwise it'll look a lot bigger than it really is.

Any questions, ask me in #🔮 on Discord (if you can't see that channel, ping Jem - see above).

NOTE: CI is not passing, because CI is not a sponsor 😉

@benjie benjie marked this pull request as draft March 9, 2023 15:22
res,
},
},
pgl.getResolvedPreset()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hookArgs is how the context is derived now, it means withPostGraphileContextFromReqRes is no longer needed. We pass the req/res objects (both into expressv4 and node scopes) so that hookArgs can use them to call your context callback (or pgSettings/additionalGraphQLContextFromRequest if you're using the V4 preset, which we are) to derive the GraphQL context.

any,
any,
any
>;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TypeScript types in V5 need improving so that this cast isn't needed 😅

return {
data: row,
};
return {};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's now the responsibility of the next layer of plans to execute this. See the LoginPayload.user plan above, which is effectively:

    LoginPayload: {
      user() {
        const $userId = currentUserIdSource.execute();
        return userSource.get({ id: $userId });
      },
    }

!s.parameters &&
s.extensions?.pg?.schemaName === "app_public" &&
s.extensions.pg.name === "users"
);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an alternative approach if you know the database name rather than the inflected source name. I include it as an alternative to the much simpler equivalent code we saw before:

const userSource = build.input.pgSources.find((s) => s.name === "users");

currentUserUpdated: UserSubscriptionPayload @pgSubscription(topic: ${embed(
currentUserTopicFromContext
)})
currentUserUpdated: UserSubscriptionPayload
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @pgSubscription directive is no more, instead write a subscribePlan (see below).

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.

3 participants