Skip to content

Add full runtime support for node-redis (v4/v5) clients - #714

Open
rafagsiqueira wants to merge 11 commits into
davidyaha:masterfrom
rafagsiqueira:master
Open

Add full runtime support for node-redis (v4/v5) clients#714
rafagsiqueira wants to merge 11 commits into
davidyaha:masterfrom
rafagsiqueira:master

Conversation

@rafagsiqueira

Copy link
Copy Markdown

Summary

Completes the work started in #712, which added TypeScript types for node-redis clients but only handled compile-time typing — at runtime, node-redis's promise-based pub/sub API was invoked as if it were ioredis's callback-based API, so subscribe() never actually resolved and messages were never delivered (node-redis has no generic 'message'/'pmessage' events; it delivers messages to the listener passed directly to subscribe()/pSubscribe()).

  • RedisPubSub now detects which client library (ioredis vs node-redis) was passed in as publisher/subscriber and branches its pub/sub wiring accordingly.
  • subscribe(): for node-redis, passes a real per-trigger listener into subscribe()/pSubscribe() and awaits the returned Promise instead of misinterpreting the message listener as an error-first callback.
  • The constructor only wires the generic 'message'/'pmessage' event listeners for ioredis clients, since node-redis has no equivalent.
  • unsubscribe()/close() call the correct method names (pUnsubscribe/close vs punsubscribe/quit) and normalize return values.
  • Added redis as an optional/dev dependency, matching the existing ioredis pattern.

Test plan

  • Added two new integration suites in src/test/integration-tests.ts covering node-redis: plain client pub/sub (regular + pattern subscriptions) and node-redis Cluster pub/sub.
  • npm run testonly — 24 passing (existing ioredis-mock unit tests, unaffected).
  • npm run integration — 10 passing (existing ioredis/cluster tests + new node-redis/node-redis-cluster tests), run repeatedly against a real local Redis + Redis Cluster to confirm stability.
  • npm run lint and tsc — clean (no new warnings/errors).

🤖 Generated with Claude Code

rafaelgsiqueira and others added 10 commits July 8, 2026 17:11
The upstream graphql-redis-subscriptions project appears unmaintained.
Publish this fork under a new npm name.

- package.json: rename package, point repo/bugs/homepage at the fork,
  add contributor, restrict published files to dist/
- README: retitle, add fork note, update install + import examples
- LICENSE: retain original copyright, add fork maintainer

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Raises line coverage from 89.69% to ~96%, above nyc's default threshold.
The node-redis support and with-filter helper had no direct tests.

Also point repository/bugs/homepage at the renamed
rafagsiqueira/apollo-redis-subscriptions repo.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…riptions

Rebrand fork as apollo-redis-subscriptions
v2.7.1: version bump and local cluster test setup
- Run `npm run compile` (the actual build script) so dist/ exists;
  `npm run build --if-present` was a silent no-op, which would have
  published an empty tarball given files: ["dist"].
- Upgrade npm to guarantee >= 11.5.1 for trusted publishing (OIDC).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
setup-node's `cache: false` errors with "Caching for 'false' is not
supported"; `package-manager` is not a valid input. Omitting both
disables dependency caching, which is fine for the publish job.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`npm install -g npm@latest` left the global npm unable to resolve the
bundled `sigstore` module, breaking `npm publish --provenance`. Node 24
already ships an OIDC-capable npm, so drop the self-update.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
prepublishOnly already runs the full test suite during npm publish.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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