-
-
Notifications
You must be signed in to change notification settings - Fork 292
feat(apollo): upgrade Apollo Server and related integrations and add … #3230
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
feat(apollo): upgrade Apollo Server and related integrations and add … #3230
Conversation
WalkthroughThe PR upgrades Apollo Server to v5, introduces dynamic Express version detection to support both v4 and v5 integrations, adds optional peer/dev dependencies for Express, Koa, and Fastify integrations, updates GraphQL versions, and adds/updates tests and documentation for these changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Module as ApolloModule
participant Injector
participant Service as ApolloService
participant Platform as PlatformConfiguration
participant ExpressPkg as express/package.json
rect `#e6f7ff`
Note right of Module: $onRoutesInit
end
Module ->> Injector: $alterApolloSettings (async)
Injector -->> Module: altered settings?
Module ->> Service: createServer(id, settings)
rect `#fff4e6`
Note right of Service: runtime detection & import
end
Service ->> ExpressPkg: read version
ExpressPkg -->> Service: version string
alt major == 5
Service ->> Service: dynamic import `@as-integrations/express5`
else major == 4
Service ->> Service: dynamic import `@as-integrations/express4`
else unsupported
Service -->> Module: throw unsupported-express-version
end
Service ->> Service: apply middleware to platform (use/mount)
Module ->> Platform: $afterListen -> getBestHost
Platform -->> Module: host:port
Module ->> Module: log server URL(s)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Perfect! @timothygachengo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/graphql/apollo/src/services/ApolloService.ts (2)
70-93: Wrap switch case declarations in a block scope.Biome correctly flags that
const/letdeclarations inside switch cases without block scope can be erroneously accessed by other cases. Wrap theexpresscase body in braces to restrict variable scope.🔎 Proposed fix
switch (this.platformName) { - case "express": - // use newer graphql express middleware - const expressVersion = await import("express/package.json").then((pkg) => pkg.default.version); - const majorVersion = parseInt(expressVersion.split(".")[0], 10); - - let expressMiddleware: any; - - if (majorVersion >= 5) { - ({expressMiddleware} = await import("@as-integrations/express5")); - } else if (majorVersion === 4) { - ({expressMiddleware} = await import("@as-integrations/express4")); - } else { - throw new Error(`Express version ${expressVersion} is not supported. Please use Express 4 or 5.`); - } - - this.app.use( - path, - expressMiddleware(server as any, { - ...middlewareOptions, - context: contextHandler - }) - ); - break; + case "express": { + // use newer graphql express middleware + const expressVersion = await import("express/package.json").then((pkg) => pkg.default.version); + const majorVersion = parseInt(expressVersion.split(".")[0], 10); + + let expressMiddleware: any; + + if (majorVersion >= 5) { + ({expressMiddleware} = await import("@as-integrations/express5")); + } else if (majorVersion === 4) { + ({expressMiddleware} = await import("@as-integrations/express4")); + } else { + throw new Error(`Express version ${expressVersion} is not supported. Please use Express 4 or 5.`); + } + + this.app.use( + path, + expressMiddleware(server as any, { + ...middlewareOptions, + context: contextHandler + }) + ); + break; + }
95-105: Consider adding block scope to thekoacase for consistency.For consistency with the suggested fix for the
expresscase and to prevent potential issues if this case is extended in the future.🔎 Proposed fix
- case "koa": - const {koaMiddleware} = await import("@as-integrations/koa"); - - this.app.use( - path, - koaMiddleware(server as any, { - ...middlewareOptions, - context: contextHandler - }) - ); - break; + case "koa": { + const {koaMiddleware} = await import("@as-integrations/koa"); + + this.app.use( + path, + koaMiddleware(server as any, { + ...middlewareOptions, + context: contextHandler + }) + ); + break; + }
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
packages/graphql/apollo/package.jsonpackages/graphql/apollo/readme.mdpackages/graphql/apollo/src/ApolloModule.spec.tspackages/graphql/apollo/src/services/ApolloService.spec.tspackages/graphql/apollo/src/services/ApolloService.tspackages/graphql/typegraphql/package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prohibit absolute imports between packages within the monorepo; use relative imports instead
Files:
packages/graphql/apollo/src/ApolloModule.spec.tspackages/graphql/apollo/src/services/ApolloService.spec.tspackages/graphql/apollo/src/services/ApolloService.ts
**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place test files with
.spec.tsnaming convention and run via Vitest with per-package configurations
Files:
packages/graphql/apollo/src/ApolloModule.spec.tspackages/graphql/apollo/src/services/ApolloService.spec.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: Romakita
Repo: tsedio/tsed PR: 3148
File: packages/platform/platform-express/src/components/PlatformExpress.ts:226-230
Timestamp: 2025-08-20T20:24:35.591Z
Learning: Ts.ED framework defaults to extended query parsing behavior in v4, so maintaining "extended" query parser in Express v5 is preserving existing functionality rather than introducing new behavior that needs migration documentation.
📚 Learning: 2025-12-01T15:26:21.293Z
Learnt from: CR
Repo: tsedio/tsed PR: 0
File: docs/public/ai/AGENTS.md:0-0
Timestamp: 2025-12-01T15:26:21.293Z
Learning: Applies to docs/public/ai/src/**/*.spec.ts : Update or add unit/integration tests near changed files (test/ or src/**/**.spec.ts co-location)
Applied to files:
packages/graphql/apollo/src/ApolloModule.spec.tspackages/graphql/apollo/src/services/ApolloService.spec.ts
📚 Learning: 2025-01-29T23:19:32.209Z
Learnt from: LonguCodes
Repo: tsedio/tsed PR: 2974
File: packages/orm/objection/src/decorators/belongsToOne.spec.ts:86-111
Timestamp: 2025-01-29T23:19:32.209Z
Learning: The test suite for `BelongsToOne` decorator in `belongsToOne.spec.ts` focuses on positive cases of relationship mapping configuration. Error cases are not applicable to this test suite as they are likely handled at TypeScript compilation time or in separate validation layers.
Applied to files:
packages/graphql/apollo/src/ApolloModule.spec.ts
📚 Learning: 2025-12-01T15:26:21.293Z
Learnt from: CR
Repo: tsedio/tsed PR: 0
File: docs/public/ai/AGENTS.md:0-0
Timestamp: 2025-12-01T15:26:21.293Z
Learning: Use the Ts.ED CLI (tsed/cli) to scaffold controllers, services, DTOs, middlewares, interceptors, and tests to ensure files, names, and boilerplate match current best practices
Applied to files:
packages/graphql/apollo/readme.md
📚 Learning: 2024-11-28T13:04:43.555Z
Learnt from: Romakita
Repo: tsedio/tsed PR: 2900
File: packages/platform/platform-test-sdk/src/interfaces/PlatformTestingSdkOpts.ts:6-6
Timestamp: 2024-11-28T13:04:43.555Z
Learning: Les modifications apportées aux fichiers de test internes, tels que `packages/platform/platform-test-sdk/src/interfaces/PlatformTestingSdkOpts.ts`, sont considérées comme internes au repository et n'introduisent pas de breaking change potentiel.
Applied to files:
packages/graphql/apollo/src/services/ApolloService.spec.ts
🧬 Code graph analysis (2)
packages/graphql/apollo/src/ApolloModule.spec.ts (4)
packages/platform/platform-http/src/testing/PlatformTest.ts (1)
PlatformTest(19-165)packages/graphql/apollo/src/services/ApolloService.ts (1)
ApolloService(18-231)packages/di/src/common/services/InjectorService.ts (1)
InjectorService(54-656)packages/graphql/apollo/src/interfaces/ApolloSettings.ts (1)
ApolloSettings(22-40)
packages/graphql/apollo/src/services/ApolloService.spec.ts (2)
packages/platform/platform-http/src/testing/PlatformTest.ts (1)
PlatformTest(19-165)packages/core/src/utils/catchError.ts (1)
catchAsyncError(21-27)
🪛 Biome (2.1.2)
packages/graphql/apollo/src/services/ApolloService.ts
[error] 73-73: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 74-74: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 76-76: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 LanguageTool
packages/graphql/apollo/readme.md
[grammar] ~55-~55: Ensure spelling is correct
Context: ...e using express you need to install its respecitve integration: - Express 4 - [@as-integr...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
packages/graphql/apollo/readme.md
77-77: Spaces inside code span elements
(MD038, no-space-in-code)
🔇 Additional comments (7)
packages/graphql/typegraphql/package.json (1)
41-41: LGTM!The GraphQL version bump to 16.12.0 aligns with the broader upgrade across the monorepo. Removing
@types/graphqlis appropriate since GraphQL 16+ ships with built-in TypeScript definitions.packages/graphql/apollo/src/ApolloModule.spec.ts (1)
1-217: LGTM!Comprehensive test coverage for
ApolloModuleincluding:
- Settings resolution with proper fallback order (apollo → graphql → typegraphql)
$onRoutesInitlifecycle hook with server creation and settings alteration$afterListenhook with URL logging across various host configurations- Edge cases for missing settings
The tests correctly use relative imports and follow the
.spec.tsco-location pattern.packages/graphql/apollo/src/services/ApolloService.spec.ts (3)
19-47: LGTM on the mock structure for Express integration tests.The mock setup correctly separates
@as-integrations/express4and@as-integrations/express5with distinct return values (expressMiddleware4andexpressMiddleware5), enabling clear test assertions for each Express version path.
307-340: Good coverage for unsupported Express version error path.The test correctly verifies that
createServerthrows an appropriate error when an unsupported Express version (3.x) is detected, validating the error message content.
272-306: The test is correctly structured and does exercise the Express 5 code path as intended.The
vi.doMockandvi.doUnmockpattern in thebeforeEach/afterEachhooks is the proper Vitest way to override module mocks between tests. Whenservice.createServer("key", opts)is called within the Express 5 test, it performs a fresh dynamicimport("express/package.json")that correctly retrieves the mocked version"5.0.0", which leads to importing@as-integrations/express5and thus calling the mockedexpressMiddlewarethat returns"expressMiddleware5". The assertion expectingapp.useto be called with("/path", "expressMiddleware5")is accurate and will pass.Likely an incorrect or invalid review comment.
packages/graphql/apollo/package.json (2)
60-72: LGTM on optional peer dependencies structure.The
peerDependenciesMetacorrectly marks all integration packages as optional, allowing users to install only the integration they need (Express 4, Express 5, Koa, or Fastify).
30-30: Apollo Server v5 migration is properly implemented.The upgrade to
@apollo/serverv5 has been correctly handled. The critical breaking change—Express integration removal—is addressed via the new@as-integrations/express4and@as-integrations/express5packages added as devDependencies.ApolloService.tsdynamically imports the appropriate integration based on the Express version in use. Thegraphqldependency (16.12.0) meets the v5 requirement (≥16.11.0), no deprecated@apollo/server/express*imports remain, and all API usage follows v5 patterns.
|
@timothygachengo just fix typo hightlighted by coderabbit review ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/graphql/apollo/readme.md
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Romakita
Repo: tsedio/tsed PR: 3148
File: packages/platform/platform-express/src/components/PlatformExpress.ts:226-230
Timestamp: 2025-08-20T20:24:35.591Z
Learning: Ts.ED framework defaults to extended query parsing behavior in v4, so maintaining "extended" query parser in Express v5 is preserving existing functionality rather than introducing new behavior that needs migration documentation.
📚 Learning: 2025-12-01T15:26:21.293Z
Learnt from: CR
Repo: tsedio/tsed PR: 0
File: docs/public/ai/AGENTS.md:0-0
Timestamp: 2025-12-01T15:26:21.293Z
Learning: Use the Ts.ED CLI (tsed/cli) to scaffold controllers, services, DTOs, middlewares, interceptors, and tests to ensure files, names, and boilerplate match current best practices
Applied to files:
packages/graphql/apollo/readme.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: test-platform (20.19.4)
- GitHub Check: test-third-parties (20.19.4)
- GitHub Check: test-graphql (20.19.4)
- GitHub Check: test-security (20.19.4)
- GitHub Check: test-specs (20.19.4)
- GitHub Check: test-orm (20.19.4)
- GitHub Check: lint (20.19.4)
- GitHub Check: test-core (20.19.4)
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
packages/graphql/apollo/readme.md (1)
53-77: Excellent—all past review issues have been addressed.The Express Integration (lines 53-67), Koa Integration (lines 69-73), and Fastify placeholder (line 77) sections are now correctly structured. The spelling correction ("respective"), Express 5 package name (@as-integrations/express5), and Fastify formatting are all in place.
|
@Romakita Could you merge this for me please? |
|
🎉 This PR is included in version 8.22.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Information
This PR updates the Apollo integration in the
@tsed/apollopackage to support both Express 4 and Express 5. The most significant changes are the dynamic selection of Express middleware based on the Express version, the addition of new peer dependencies, expanded tests for different Express versions, and updated documentation for integration steps.Documentation updates:
Dependency updates:
@apollo/serverandgraphqldependencies to newer versions.Todos
Resources:
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.