feat: replace Socket.IO real-time sync with GraphQL subscriptions#118
Conversation
|
@DanielEmmanuel1 is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughReplaces Socket.IO realtime sync with GraphQL Subscriptions: adds a graphql-ws client, typed subscription documents, generic and bounty-specific subscription hooks, tests and docs; removes Socket.IO client, socket sync modules and handlers; wires subscriptions into the QueryProvider. Changes
Sequence DiagramsequenceDiagram
participant Component as React Component
participant Hook as useBountySubscription
participant WSClient as WS Client (wsClient)
participant Server as GraphQL Server
participant QueryClient as React Query
Component->>Hook: mount / useBountySubscription()
Hook->>WSClient: subscribe(BOUNTY_CREATED)
Hook->>WSClient: subscribe(BOUNTY_UPDATED)
Hook->>WSClient: subscribe(BOUNTY_DELETED)
WSClient->>Server: connect + auth (connectionParams)
Server-->>WSClient: connection ack
Server->>WSClient: bountyCreated event
WSClient->>Hook: onData(bountyCreated)
Hook->>QueryClient: invalidateQueries(bountyKeys.allListKeys / lists)
Server->>WSClient: bountyUpdated event
WSClient->>Hook: onData(bountyUpdated)
Hook->>QueryClient: invalidateQueries(bountyKeys.detail(id))
Hook->>QueryClient: invalidateQueries(bountyKeys.allListKeys / lists)
Server->>WSClient: bountyDeleted event
WSClient->>Hook: onData(bountyDeleted)
Hook->>QueryClient: removeQueries(bountyKeys.detail(id))
Hook->>QueryClient: invalidateQueries(bountyKeys.allListKeys / lists)
Component->>Hook: unmount
Hook->>WSClient: unsubscribe(all)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
hooks/use-bounty-subscription.ts (1)
21-36: Inline{}literals and arrow callbacks will cause subscription churn — addressed by the fix proposed inhooks/use-graphql-subscription.ts.Each render creates new object/function references for
variablesand theonDatacallbacks, which propagates to theuseEffectdep array inuseGraphQLSubscriptionand triggers a full unsubscribe+resubscribe cycle. The fix inuse-graphql-subscription.ts(ref-based callbacks + stablevariablesarg) resolves this here too. See proposed fix inuse-graphql-subscription.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-bounty-subscription.ts` around lines 21 - 36, The subscription usages create new object and function references each render (empty {} and inline arrow callbacks), causing churn; update the three calls to useGraphQLSubscription in use-bounty-subscription.ts to pass stable variables (e.g., reuse a shared const variables or undefined) and provide stable callbacks (either by moving the handlers into named functions like handleBountyCreated, handleBountyUpdated, handleBountyDeleted declared outside the render or by wrapping them with useRef/useCallback as per the change in use-graphql-subscription.ts); ensure the handlers call queryClient.invalidateQueries/bountyKeys.detail/removeQueries as before but are stable references so the subscription hook does not unsubscribe/resubscribe every render.
🧹 Nitpick comments (4)
hooks/__tests__/use-bounty-subscription.test.ts (1)
32-38:gqlmock parameter should be typed asTemplateStringsArray, notstring[].
gqlis a tagged template literal; its first argument isTemplateStringsArray. WhileTemplateStringsArray extends ReadonlyArray<string>makes it work at runtime, the declared type is incorrect.♻️ Proposed fix
jest.mock('graphql-tag', () => ({ - gql: (strings: string[]) => ({ + gql: (strings: TemplateStringsArray) => ({ kind: 'Document', definitions: [], loc: { source: { body: strings[0].trim() } }, }), }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/__tests__/use-bounty-subscription.test.ts` around lines 32 - 38, The mocked gql signature uses the wrong parameter type; change the mocked function in the jest.mock call so its first parameter is typed as TemplateStringsArray instead of string[] (i.e., update the gql: (strings: string[]) => ... to gql: (strings: TemplateStringsArray) => ...), keeping the same return shape; this will correctly reflect that gql is a tagged template literal and satisfy TypeScript.providers/query-provider.tsx (1)
44-49: Move the import to the top of the file.The
useBountySubscriptionimport at Line 44 is placed after theQueryProviderPropsinterface declaration, breaking the file's import block.♻️ Proposed fix
"use client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { ReactQueryDevtools } from "@tanstack/react-query-devtools"; import { useState, type ReactNode } from "react"; import { QueryErrorBoundary } from "./query-error-boundary"; +import { useBountySubscription } from "@/hooks/use-bounty-subscription"; function makeQueryClient(): QueryClient { ... interface QueryProviderProps { children: ReactNode; } -import { useBountySubscription } from "@/hooks/use-bounty-subscription"; - function RealtimeSync() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@providers/query-provider.tsx` around lines 44 - 49, Move the useBountySubscription import so all imports are grouped at the top of the module: relocate the line importing useBountySubscription to the top import block (above the QueryProviderProps interface declaration) and ensure RealtimeSync still calls useBountySubscription(); this keeps the import ordering consistent and prevents breaking the file-level import block for the QueryProviderProps interface and RealtimeSync function.lib/graphql/subscriptions.ts (1)
44-68: Manual payload interfaces bypass the existinggraphql-codegensetup and can silently drift from the server schema.The project already uses
@graphql-codegen/typescript-operations(present indevDependenciesand thecodegen/codegen:watchscripts). Running codegen against the subscriptions defined here would auto-generateBountyCreatedData,BountyUpdatedData, andBountyDeletedDatafrom the actual server schema, keeping them in sync automatically. The current hand-written interfaces will not be updated if the server schema changes.Consider adding these subscription documents to the codegen input glob so the corresponding types are generated automatically, then import from the generated types file instead of maintaining these manually.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/graphql/subscriptions.ts` around lines 44 - 68, Remove the hand-written interfaces BountyCreatedData, BountyUpdatedData, and BountyDeletedData and instead add the corresponding subscription GraphQL documents to the codegen input glob so `@graphql-codegen/typescript-operations` generates these types; then import the generated types (e.g., BountyCreatedData, BountyUpdatedData, BountyDeletedData) from the codegen output file (the types index) and replace the manual definitions with those imports in subscriptions.ts to keep types in sync with the server schema.lib/graphql/ws-client.ts (1)
17-18:shouldRetry: () => trueretries auth failures exhausting all 5 attempts before surfacing the error.
graphql-wspasses the close event toshouldRetry. Auth rejections from the server (e.g.,4400/4401/4403close codes) are not transient, so retrying them wastes cycles and delays meaningful error feedback. Consider gating retries on the error type:♻️ Proposed fix
- retryAttempts: 5, - shouldRetry: () => true, + retryAttempts: 5, + shouldRetry: (errOrCloseEvent) => { + // Do not retry on auth-related close codes + if ( + errOrCloseEvent instanceof CloseEvent && + [4400, 4401, 4403].includes(errOrCloseEvent.code) + ) { + return false; + } + return true; + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/graphql/ws-client.ts` around lines 17 - 18, Update the retry logic so auth-related close codes don't trigger retries: in the WebSocket client config where retryAttempts and shouldRetry are defined (the shouldRetry: () => true entry), change shouldRetry to accept the close event/error passed by graphql-ws and return false for non-transient authentication close codes (at least 4400, 4401, 4403), and otherwise allow retries (subject to retryAttempts: 5); locate the shouldRetry callback in lib/graphql/ws-client.ts and implement the conditional check on the close event.code to skip retries for those auth failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/__tests__/use-bounty-subscription.test.ts`:
- Around line 70-73: The test uses unsafe "as any" casts for subscription
constants; replace them with proper typed assertions by importing DocumentNode
from 'graphql', creating typed intermediates (e.g., const created =
BOUNTY_CREATED_SUBSCRIPTION as DocumentNode) for BOUNTY_CREATED_SUBSCRIPTION,
BOUNTY_UPDATED_SUBSCRIPTION, and BOUNTY_DELETED_SUBSCRIPTION, then use
created.loc?.source?.body (and the equivalents for updated/deleted) when
building subscribedQueries to assert against mockSubscribe.mock.calls; keep
mockSubscribe and subscribedQueries logic unchanged.
- Around line 41-43: The mock for graphql.print currently types its parameter as
any which fails CI; update the jest.mock implementation to type the parameter as
DocumentNode (import DocumentNode from 'graphql') or a minimal shape like { loc:
{ source: { body: string } } } and adjust the mock signature in the print stub
accordingly so it reads e.g. print: jest.fn((query: DocumentNode) =>
query.loc.source.body) (or use the minimal inline shape) to satisfy type
checking while keeping the same return behavior.
In `@hooks/use-graphql-subscription.ts`:
- Around line 21-40: The effect is re-subscribing on every render because
variables and the callbacks change identity; in useEffect where
wsClient.subscribe is called (note queryString, variables, onData, onError and
unsubscribe), stop passing unstable objects/callbacks into the dependency array:
keep query (or printed queryString) and stable identifiers only, move onData and
onError into refs (e.g., onDataRef/onErrorRef) and call the latest callbacks
from the subscription handlers, and ensure variables is stabilized by requiring
callers to memoize it (useMemo) or by storing a stableRef of variables and
comparing shallow equality before re-subscribing; update the dependency array to
use queryString (or query) and any truly stable values only so subscriptions
aren’t torn down on each render.
---
Duplicate comments:
In `@hooks/use-bounty-subscription.ts`:
- Around line 21-36: The subscription usages create new object and function
references each render (empty {} and inline arrow callbacks), causing churn;
update the three calls to useGraphQLSubscription in use-bounty-subscription.ts
to pass stable variables (e.g., reuse a shared const variables or undefined) and
provide stable callbacks (either by moving the handlers into named functions
like handleBountyCreated, handleBountyUpdated, handleBountyDeleted declared
outside the render or by wrapping them with useRef/useCallback as per the change
in use-graphql-subscription.ts); ensure the handlers call
queryClient.invalidateQueries/bountyKeys.detail/removeQueries as before but are
stable references so the subscription hook does not unsubscribe/resubscribe
every render.
---
Nitpick comments:
In `@hooks/__tests__/use-bounty-subscription.test.ts`:
- Around line 32-38: The mocked gql signature uses the wrong parameter type;
change the mocked function in the jest.mock call so its first parameter is typed
as TemplateStringsArray instead of string[] (i.e., update the gql: (strings:
string[]) => ... to gql: (strings: TemplateStringsArray) => ...), keeping the
same return shape; this will correctly reflect that gql is a tagged template
literal and satisfy TypeScript.
In `@lib/graphql/subscriptions.ts`:
- Around line 44-68: Remove the hand-written interfaces BountyCreatedData,
BountyUpdatedData, and BountyDeletedData and instead add the corresponding
subscription GraphQL documents to the codegen input glob so
`@graphql-codegen/typescript-operations` generates these types; then import the
generated types (e.g., BountyCreatedData, BountyUpdatedData, BountyDeletedData)
from the codegen output file (the types index) and replace the manual
definitions with those imports in subscriptions.ts to keep types in sync with
the server schema.
In `@lib/graphql/ws-client.ts`:
- Around line 17-18: Update the retry logic so auth-related close codes don't
trigger retries: in the WebSocket client config where retryAttempts and
shouldRetry are defined (the shouldRetry: () => true entry), change shouldRetry
to accept the close event/error passed by graphql-ws and return false for
non-transient authentication close codes (at least 4400, 4401, 4403), and
otherwise allow retries (subject to retryAttempts: 5); locate the shouldRetry
callback in lib/graphql/ws-client.ts and implement the conditional check on the
close event.code to skip retries for those auth failures.
In `@providers/query-provider.tsx`:
- Around line 44-49: Move the useBountySubscription import so all imports are
grouped at the top of the module: relocate the line importing
useBountySubscription to the top import block (above the QueryProviderProps
interface declaration) and ensure RealtimeSync still calls
useBountySubscription(); this keeps the import ordering consistent and prevents
breaking the file-level import block for the QueryProviderProps interface and
RealtimeSync function.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
README.mddocs/REALTIME_SYNC.mdhooks/__tests__/use-bounty-subscription.test.tshooks/use-bounty-subscription.tshooks/use-graphql-subscription.tshooks/use-socket-sync.tslib/graphql/subscriptions.tslib/graphql/ws-client.tslib/query/sync/handlers.tslib/query/sync/socket-sync.tslib/socket/client.tspackage.jsonproviders/query-provider.tsx
💤 Files with no reviewable changes (4)
- hooks/use-socket-sync.ts
- lib/query/sync/handlers.ts
- lib/socket/client.ts
- lib/query/sync/socket-sync.ts
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (7)
hooks/use-graphql-subscription.ts (1)
22-28: Ref update insideuseEffectintroduces a stale-callback window — update refs during render instead.Lines 25–28 update
onDataRef/onErrorRefin an effect, which runs asynchronously after the render commit. If a subscription message arrives in the brief gap between the render and the effect flush,onDataRef.currentstill holds the previous callback. The idiomatic "latest-ref" pattern assigns refs synchronously in the render body:♻️ Proposed fix
- // Hold latest callbacks in refs so we don't restart the subscription when they change const onDataRef = useRef(onData); const onErrorRef = useRef(onError); - useEffect(() => { - onDataRef.current = onData; - onErrorRef.current = onError; - }, [onData, onError]); + // Always keep refs pointing at the latest callbacks (synchronous during render) + onDataRef.current = onData; + onErrorRef.current = onError;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-graphql-subscription.ts` around lines 22 - 28, The refs onDataRef and onErrorRef are updated inside a useEffect, leaving a window where incoming subscription messages may see stale callbacks; move the assignments to run synchronously during render by assigning onDataRef.current = onData and onErrorRef.current = onError directly in the component body (remove or keep useEffect only if it still serves other purposes) so the latest callbacks are always available to the subscription handlers in use-graphql-subscription.ts.lib/graphql/subscriptions.ts (1)
1-1: Usegqlfromgraphqlinstead ofgraphql-tag.Since
graphqlis already a dependency and exportsgqlsince v16,graphql-tagcan be dropped entirely.♻️ Proposed refactor
-import { gql } from 'graphql-tag'; +import { gql } from 'graphql';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/graphql/subscriptions.ts` at line 1, Replace the import of gql from 'graphql-tag' with the gql export from the 'graphql' package: locate the import statement that currently reads "import { gql } from 'graphql-tag';" and change it to import gql from the 'graphql' package's exported gql (i.e., use the 'gql' symbol from 'graphql') and remove the 'graphql-tag' dependency from package.json if it's only used here; ensure any other references to gql in this module (the gql symbol) continue to work with the new import.package.json (1)
58-58:graphql-tagis redundant —graphqlalready exportsgql.
graphqlv16 (already a dependency at line 56) re-exports thegqltag viaimport { gql } from 'graphql', makinggraphql-tagan unnecessary extra dependency. The import inlib/graphql/subscriptions.tscan be changed tographqldirectly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 58, Remove the unnecessary graphql-tag dependency from package.json and update the subscription file to use the gql exported by graphql: in lib/graphql/subscriptions.ts replace any import from 'graphql-tag' (e.g. default or named import of gql) with "import { gql } from 'graphql'"; then run your package manager to refresh node_modules and lockfile so graphql-tag is removed from the lockfile as well.providers/query-provider.tsx (1)
44-44: Misplaced import — move to top of file with other imports.The
useBountySubscriptionimport at line 44 is placed after theQueryProviderPropsinterface declaration. While JS/TS hoisting makes this functionally correct, it's non-idiomatic and can confuse readers scanning the import section.♻️ Proposed fix
import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { ReactQueryDevtools } from "@tanstack/react-query-devtools"; import { useState, type ReactNode } from "react"; import { QueryErrorBoundary } from "./query-error-boundary"; +import { useBountySubscription } from "@/hooks/use-bounty-subscription"; function makeQueryClient(): QueryClient { ... interface QueryProviderProps { children: ReactNode; } - -import { useBountySubscription } from "@/hooks/use-bounty-subscription"; function RealtimeSync() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@providers/query-provider.tsx` at line 44, The import for useBountySubscription is placed after the QueryProviderProps interface; move the line importing useBountySubscription up into the main import block with the other imports at the top of the file so all imports are grouped together (update providers/query-provider.tsx by relocating the useBountySubscription import above the QueryProviderProps declaration and ensure any linter import-order rules are satisfied).hooks/__tests__/use-bounty-subscription.test.ts (1)
45-155: No test coverage for the subscription error path.All five tests exercise the happy path and cleanup. There is no test verifying behavior when
wsClient.subscribetriggers itserrorhandler — specifically thatconsole.erroris called and an optionalonErrorcallback is invoked (as implemented inhooks/use-graphql-subscription.ts, lines 41–44). This is a meaningful coverage gap since error handling is part of the public contract of the hook.🧪 Suggested additional test
it('should log and surface errors from a subscription', () => { const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); renderHook(() => useBountySubscription()); const call = mockSubscribe.mock.calls[0]; // any subscription const errorHandler = call[1].error; const testError = new Error('WebSocket error'); errorHandler(testError); expect(consoleSpy).toHaveBeenCalledWith('[GraphQL Subscription] Error:', testError); consoleSpy.mockRestore(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/__tests__/use-bounty-subscription.test.ts` around lines 45 - 155, Missing test coverage for the subscription error path: add a test in hooks/__tests__/use-bounty-subscription.test.ts that renders useBountySubscription, spies on console.error, locates a subscription call from the mocked wsClient.subscribe (mockSubscribe.mock.calls), extracts the error handler (call[1].error), invokes it with a test Error, and asserts console.error was called with "[GraphQL Subscription] Error:" and the error and (if applicable) that any passed onError callback from use-graphql-subscription.ts was invoked; reference useBountySubscription, wsClient.subscribe (mockSubscribe), and the error handler implemented in use-graphql-subscription.ts (lines ~41–44) when locating where to trigger and assert.docs/REALTIME_SYNC.md (2)
34-39: Documentwss://for non-local environments.Line 35/38 currently normalizes
ws://, which is fine for localhost but unsafe/inapplicable in production behind TLS. Please explicitly documentwss://for deployed environments to prevent insecure or broken setup defaults.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/REALTIME_SYNC.md` around lines 34 - 39, Update the REALTIME_SYNC docs to document both schemes: keep the local default NEXT_PUBLIC_GRAPHQL_WS_URL=ws://localhost:4000/graphql for development but explicitly instruct using wss:// (e.g., wss://your-backend-url/graphql) for deployed environments behind TLS; mention that ws:// is only for local/unsecured setups and that production must use wss:// to avoid insecure or broken connections.
42-42: ClarifyconnectionParamswording to avoid protocol confusion.Line 42 says the token is injected "through the
authorizationheader withinconnectionParams". Ingraphql-ws,connectionParamsis payload data sent in theConnectionInitmessage, not a WebSocket header map. Remove "header" to clarify: the authorization token is included as a property withinconnectionParamspayload, not as an HTTP-style header. This prevents implementation drift and protocol misunderstanding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/REALTIME_SYNC.md` at line 42, Update the wording to avoid implying HTTP headers: change the sentence describing how the token is sent by `wsClient` so it states the access token is included as a property in the `connectionParams` payload (e.g., an `authorization` field) sent in the GraphQL WebSocket ConnectionInit message, rather than calling it an "authorization header"; mention `wsClient`, `connectionParams`, and the `authorization` property to make locating and editing the sentence straightforward.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/REALTIME_SYNC.md`:
- Around line 34-39: Update the REALTIME_SYNC docs to document both schemes:
keep the local default NEXT_PUBLIC_GRAPHQL_WS_URL=ws://localhost:4000/graphql
for development but explicitly instruct using wss:// (e.g.,
wss://your-backend-url/graphql) for deployed environments behind TLS; mention
that ws:// is only for local/unsecured setups and that production must use
wss:// to avoid insecure or broken connections.
- Line 42: Update the wording to avoid implying HTTP headers: change the
sentence describing how the token is sent by `wsClient` so it states the access
token is included as a property in the `connectionParams` payload (e.g., an
`authorization` field) sent in the GraphQL WebSocket ConnectionInit message,
rather than calling it an "authorization header"; mention `wsClient`,
`connectionParams`, and the `authorization` property to make locating and
editing the sentence straightforward.
In `@hooks/__tests__/use-bounty-subscription.test.ts`:
- Around line 45-155: Missing test coverage for the subscription error path: add
a test in hooks/__tests__/use-bounty-subscription.test.ts that renders
useBountySubscription, spies on console.error, locates a subscription call from
the mocked wsClient.subscribe (mockSubscribe.mock.calls), extracts the error
handler (call[1].error), invokes it with a test Error, and asserts console.error
was called with "[GraphQL Subscription] Error:" and the error and (if
applicable) that any passed onError callback from use-graphql-subscription.ts
was invoked; reference useBountySubscription, wsClient.subscribe
(mockSubscribe), and the error handler implemented in
use-graphql-subscription.ts (lines ~41–44) when locating where to trigger and
assert.
In `@hooks/use-graphql-subscription.ts`:
- Around line 22-28: The refs onDataRef and onErrorRef are updated inside a
useEffect, leaving a window where incoming subscription messages may see stale
callbacks; move the assignments to run synchronously during render by assigning
onDataRef.current = onData and onErrorRef.current = onError directly in the
component body (remove or keep useEffect only if it still serves other purposes)
so the latest callbacks are always available to the subscription handlers in
use-graphql-subscription.ts.
In `@lib/graphql/subscriptions.ts`:
- Line 1: Replace the import of gql from 'graphql-tag' with the gql export from
the 'graphql' package: locate the import statement that currently reads "import
{ gql } from 'graphql-tag';" and change it to import gql from the 'graphql'
package's exported gql (i.e., use the 'gql' symbol from 'graphql') and remove
the 'graphql-tag' dependency from package.json if it's only used here; ensure
any other references to gql in this module (the gql symbol) continue to work
with the new import.
In `@package.json`:
- Line 58: Remove the unnecessary graphql-tag dependency from package.json and
update the subscription file to use the gql exported by graphql: in
lib/graphql/subscriptions.ts replace any import from 'graphql-tag' (e.g. default
or named import of gql) with "import { gql } from 'graphql'"; then run your
package manager to refresh node_modules and lockfile so graphql-tag is removed
from the lockfile as well.
In `@providers/query-provider.tsx`:
- Line 44: The import for useBountySubscription is placed after the
QueryProviderProps interface; move the line importing useBountySubscription up
into the main import block with the other imports at the top of the file so all
imports are grouped together (update providers/query-provider.tsx by relocating
the useBountySubscription import above the QueryProviderProps declaration and
ensure any linter import-order rules are satisfied).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
README.mddocs/REALTIME_SYNC.mdhooks/__tests__/use-bounty-subscription.test.tshooks/use-bounty-subscription.tshooks/use-graphql-subscription.tshooks/use-socket-sync.tslib/graphql/subscriptions.tslib/graphql/ws-client.tslib/query/sync/handlers.tslib/query/sync/socket-sync.tslib/socket/client.tspackage.jsonproviders/query-provider.tsx
💤 Files with no reviewable changes (4)
- lib/query/sync/handlers.ts
- lib/socket/client.ts
- lib/query/sync/socket-sync.ts
- hooks/use-socket-sync.ts
0xdevcollins
left a comment
There was a problem hiding this comment.
PR thoroughly migrates real-time sync to GraphQL subscriptions. All requirements are met, code is clean and well-tested. Please just ensure cache invalidation covers all list variants.
Closes #105
Implementation Summary
lib/graphql/ws-client.tswith Bearer token authentication inconnectionParams.hooks/use-graphql-subscription.tsto manage subscription lifecycle.bountyCreated,bountyUpdated, andbountyDeletedevents.Cleanup
lib/socket/client.ts,hooks/use-socket-sync.ts,lib/query/sync/socket-sync.ts, andlib/query/sync/handlers.ts.Verification
hooks/__tests__/use-bounty-subscription.test.tsverifying subscription lifecycle and cache invalidation.docs/REALTIME_SYNC.md.Summary by CodeRabbit
New Features
Documentation
Tests
Chores