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

chore(server): apollo server v3 -> v4 #2880

Merged
merged 5 commits into from
Sep 5, 2024
Merged

chore(server): apollo server v3 -> v4 #2880

merged 5 commits into from
Sep 5, 2024

Conversation

fabis94
Copy link
Contributor

@fabis94 fabis94 commented Sep 5, 2024

there were many breaking changes in this one, the most annoying one being that the context builder function is no longer associated with the ApolloServer instance

also had to update graphql 15 -> 16

i tested out server (& tests), frontend 2 and frontend, tried out both normal operations and subscriptions - all seemed to work

Copy link

linear bot commented Sep 5, 2024

ApolloServer,
ForbiddenError,
ApolloServerExpressConfig,
ApolloError
Copy link
Contributor Author

@fabis94 fabis94 Sep 5, 2024

Choose a reason for hiding this comment

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

Apollo's ApolloError & ForbiddenError no longer exist

@@ -55,7 +55,7 @@ const cleanup = async () => {
}

describe('Stream access requests', () => {
let apollo: ApolloServer
let apollo: ServerAndContext
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Biggest annoyance - Apollo Server instance is no longer associated w/ the context builder, its now fed into the express middleware instead. When running operations against it manually, the context value has to be passed into the execute() call

import { Nullable } from '@speckle/shared'
import { UserInputError } from 'apollo-server-errors'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

UserInputError no longer exists

Copy link
Contributor

@iainsproat iainsproat left a comment

Choose a reason for hiding this comment

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

Some changes that conflict with the improvements to graphql errors I have in another PR. Let's merge this and I'll update the other PR to suit.

@fabis94 fabis94 merged commit c92938e into main Sep 5, 2024
23 of 26 checks passed
@fabis94 fabis94 deleted the fabians/CXPLA-49 branch September 5, 2024 09:27
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.

2 participants