Accept and support the upcoming Hive PubSub #8716
Conversation
|
Warning Rate limit exceeded@enisdenjo has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 24 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a HivePubSub compatibility layer and adapter (MeshFromHivePubSub, toMeshPubSub, isHivePubSub), widens many pubsub-typed APIs to accept Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Module as Module (cache/plugin/transport)
participant Types as Types Utils
participant PubSub as PubSub (Hive or Mesh)
participant MeshPS as MeshPubSub (normalized)
Caller->>Module: init({ pubsub: PubSub? })
Module->>Types: toMeshPubSub(PubSub)
alt input is HivePubSub
Types->>MeshPS: MeshFromHivePubSub.from(HivePubSub)
else input is MeshPubSub
Types-->>MeshPS: return MeshPubSub
end
Module->>MeshPS: subscribe / publish / asyncIterator(...)
MeshPS-->>Module: events / confirmations
sequenceDiagram
autonumber
actor Resolver
participant PS as pubsub param
participant Types as Types Utils
Resolver->>Types: isHivePubSub(PS)?
alt HivePubSub
Resolver->>PS: subscribe(topic) -> obtain iterator via [Symbol.asyncIterator]()
else MeshPubSub
Resolver->>PS: asyncIterator(topic)
end
Resolver->>Resolver: consume iterator
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
packages/transports/neo4j/src/executor.ts (1)
6-6: LGTM: Neo4j executor now supports HivePubSub via normalization
- Importing toMeshPubSub and widening pubsub types in the options/interfaces is consistent with the PR.
- Converting the union to a MeshPubSub before wiring features avoids downstream breakages.
- Conditional features block correctly remains undefined when no pubsub is provided.
Minor nit: the local name meshOrHivePubSub is explicit but a bit verbose; pubsub would be fine since conversion happens immediately.
Also applies to: 23-28, 121-135, 136-141
packages/legacy/types/src/index.ts (2)
100-101: Nit: Typo in variable name (“Hibe” → “Hive”).Rename for clarity and consistency.
Apply this diff:
-const meshForHibePubSub = new WeakMap<HivePubSub, MeshPubSub>(); +const meshForHivePubSub = new WeakMap<HivePubSub, MeshPubSub>(); @@ - let hivePubsub = meshForHibePubSub.get(pubsub); + let hivePubsub = meshForHivePubSub.get(pubsub); @@ - meshForHibePubSub.set(pubsub, hivePubsub); + meshForHivePubSub.set(pubsub, hivePubsub);Also applies to: 117-123
110-116: toMeshPubSub: Solid conversion and caching; add a tiny safety/clarity tweak.With the isHivePubSub fix, undefined is handled correctly. Consider an early-return for undefined to make the control flow explicit (optional).
Apply this diff:
export function toMeshPubSub(pubsub?: MeshPubSub | HivePubSub | undefined): MeshPubSub | undefined { + if (pubsub == null) { + return undefined; + } if (isHivePubSub(pubsub)) { let hivePubsub = meshForHivePubSub.get(pubsub); if (hivePubsub) { return hivePubsub; } hivePubsub = HiveMeshPubSub.from(pubsub); meshForHivePubSub.set(pubsub, hivePubsub); return hivePubsub; } return pubsub; }Also applies to: 121-126
packages/plugins/live-query/src/index.ts (1)
22-23: Avoid possible undefined type on pubsub by normalizing in one expression.Current typing can still flag pubsub as possibly undefined after toMeshPubSub. Consolidate the defaulting and conversion to keep types tight and remove the need for non-null assertions.
Apply this diff:
- options.logger ||= new DefaultLogger(); - options.pubsub ||= new PubSub(); + options.logger ||= new DefaultLogger(); @@ - const pubsub = toMeshPubSub(options.pubsub); + const pubsub = toMeshPubSub(options.pubsub ?? new PubSub());This ensures pubsub is MeshPubSub at compile-time and run-time when no pubsub is provided.
Also applies to: 25-27, 46-47
packages/legacy/utils/src/resolve-additional-resolvers.ts (1)
161-163: Explicitly handle missing pubsub to avoid runtime errors.When neither context.pubsub nor function-level pubsub is provided, ps becomes undefined and calling asyncIterator will throw. Provide a clear error to aid users.
Apply this diff:
- const ps = context.pubsub || pubsub; - if (isHivePubSub(ps)) { + const ps = context.pubsub || pubsub; + if (!ps) { + throw new Error( + 'PubSub instance is required to use "pubsubTopic" in additional resolvers. ' + + 'Provide it via Mesh context or resolveAdditionalResolvers(.., pubsub).', + ); + } + if (isHivePubSub(ps)) { return ps.subscribe(topic)[Symbol.asyncIterator](); } return ps.asyncIterator(topic)[Symbol.asyncIterator]();Note: This also relies on the isHivePubSub null-safe fix in types.
Also applies to: 168-182
packages/cache/redis/src/index.ts (1)
124-129: Fix potential TS type error and guard unsubscribe.Inside the destroy callback, pubsub may be typed as possibly undefined and id may be undefined. Guard both to satisfy types and avoid accidental runtime issues.
Apply this diff:
- const id = pubsub?.subscribe('destroy', () => { - this.client.disconnect(false); - pubsub.unsubscribe(id); - }); + const id = pubsub?.subscribe('destroy', () => { + this.client.disconnect(false); + if (id != null) { + pubsub?.unsubscribe(id); + } + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
.changeset/@graphql-mesh_types-8716-dependencies.md(1 hunks).changeset/cute-ducks-brush.md(1 hunks).changeset/hungry-suns-appear.md(1 hunks)packages/cache/inmemory-lru/src/index.ts(1 hunks)packages/cache/localforage/src/index.ts(2 hunks)packages/cache/redis/src/index.ts(3 hunks)packages/legacy/types/package.json(1 hunks)packages/legacy/types/src/index.ts(2 hunks)packages/legacy/utils/src/resolve-additional-resolvers.ts(4 hunks)packages/loaders/json-schema/src/types.ts(2 hunks)packages/loaders/neo4j/src/schema.ts(2 hunks)packages/plugins/live-query/src/index.ts(4 hunks)packages/plugins/live-query/src/useInvalidateByResult.ts(3 hunks)packages/transports/neo4j/src/executor.ts(3 hunks)packages/transports/rest/src/directives/process.ts(2 hunks)packages/transports/rest/src/directives/pubsubOperation.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-31T09:44:11.443Z
Learnt from: ardatan
PR: ardatan/graphql-mesh#8350
File: packages/cache/upstash-redis/src/index.ts:26-34
Timestamp: 2025-01-31T09:44:11.443Z
Learning: In GraphQL Mesh's KeyValueCache implementation, the `options.ttl` parameter in the `set` method is specified in seconds, which needs to be converted to milliseconds when using Redis's `px` option.
Applied to files:
packages/cache/inmemory-lru/src/index.ts
🧬 Code Graph Analysis (11)
packages/loaders/json-schema/src/types.ts (1)
packages/legacy/types/src/index.ts (2)
MeshPubSub(77-87)HivePubSub(89-89)
packages/transports/rest/src/directives/process.ts (1)
packages/legacy/types/src/index.ts (2)
MeshPubSub(77-87)HivePubSub(89-89)
packages/loaders/neo4j/src/schema.ts (1)
packages/legacy/types/src/index.ts (1)
MeshPubSub(77-87)
packages/cache/inmemory-lru/src/index.ts (1)
packages/legacy/types/src/index.ts (4)
MeshPubSub(77-87)HivePubSub(89-89)KeyValueCache(39-44)toMeshPubSub(115-126)
packages/legacy/utils/src/resolve-additional-resolvers.ts (1)
packages/legacy/types/src/index.ts (3)
MeshPubSub(77-87)HivePubSub(89-89)isHivePubSub(95-98)
packages/cache/localforage/src/index.ts (2)
packages/legacy/types/src/index.ts (1)
MeshPubSub(77-87)packages/legacy/types/src/config.ts (1)
LocalforageConfig(1828-1838)
packages/plugins/live-query/src/useInvalidateByResult.ts (1)
packages/legacy/types/src/index.ts (3)
MeshPubSub(77-87)HivePubSub(89-89)toMeshPubSub(115-126)
packages/plugins/live-query/src/index.ts (1)
packages/legacy/types/src/index.ts (3)
MeshPubSub(77-87)HivePubSub(89-89)toMeshPubSub(115-126)
packages/transports/neo4j/src/executor.ts (1)
packages/legacy/types/src/index.ts (3)
MeshPubSub(77-87)HivePubSub(89-89)toMeshPubSub(115-126)
packages/cache/redis/src/index.ts (2)
packages/legacy/types/src/index.ts (4)
MeshPubSub(77-87)HivePubSub(89-89)Logger(259-268)toMeshPubSub(115-126)packages/legacy/types/src/config.ts (1)
Cache(1807-1816)
packages/transports/rest/src/directives/pubsubOperation.ts (1)
packages/legacy/types/src/index.ts (3)
MeshPubSub(77-87)HivePubSub(89-89)toMeshPubSub(115-126)
🪛 LanguageTool
.changeset/@graphql-mesh_types-8716-dependencies.md
[grammar] ~4-~4: There might be a mistake here.
Context: .../types": patch --- dependencies updates: - Added dependency [`@graphql-hive/pubsub@...
(QB_NEW_EN)
[grammar] ~5-~5: There might be a mistake here.
Context: ...hive/pubsub/v/2.0.0) (to dependencies)
(QB_NEW_EN)
🔇 Additional comments (17)
.changeset/hungry-suns-appear.md (1)
1-5: LGTM: concise, accurate changeset entry.Clearly communicates the new HivePubSub export and conversion utility. No action needed.
.changeset/@graphql-mesh_types-8716-dependencies.md (1)
4-5: Nit: fix grammar and correct the npm link target.
- “dependencies updates:” → “Dependency updates:”
- The link currently points to v/2.0.0; update it to the exact alpha version page.
Apply this diff:
- dependencies updates: - - Added dependency [`@graphql-hive/[email protected]` ↗︎](https://www.npmjs.com/package/@graphql-hive/pubsub/v/2.0.0) (to `dependencies`) + Dependency updates: + - Added dependency [`@graphql-hive/[email protected]` ↗︎](https://www.npmjs.com/package/@graphql-hive/pubsub/v/2.0.0-alpha-86d532fca168f7b6e57de46d3cff2439cc6242b7) (to `dependencies`).changeset/cute-ducks-brush.md (1)
13-13: LGTM: clear user-facing note about HivePubSub support.Message is concise and consistent with the rest of the PR.
packages/loaders/neo4j/src/schema.ts (2)
10-10: LGTM: type-only import for HivePubSub inclusion.Using
import typeavoids runtime overhead and widens typing cleanly.
54-54: Downstream already accepts the widened unionVerified that in packages/transports/neo4j/src/executor.ts, getExecutableSchemaFromTypeDefsAndDriver’s signature declares
pubsub?: MeshPubSub | HivePubSub;so the broadened type is already accepted downstream. No further changes needed.
packages/loaders/json-schema/src/types.ts (1)
6-6: Type widening to support HivePubSub looks goodImporting HivePubSub and widening the pubsub type union in JSONSchemaLoaderOptions is consistent with the PR’s intent and other modules.
Also applies to: 17-17
packages/plugins/live-query/src/useInvalidateByResult.ts (1)
59-59: LGTM: publishing via normalized pubsubSwitching to publish through the normalized pubsub is correct and consistent with the PR’s pattern.
packages/cache/localforage/src/index.ts (1)
3-9: LGTM: constructor accepts HivePubSub and forwards to fallbackWidening the pubsub type in the constructor and forwarding it to InMemoryLRUCache for the fallback path aligns with the overall normalization approach. No runtime behavior change otherwise.
Also applies to: 13-17
packages/cache/inmemory-lru/src/index.ts (1)
11-15: LGTM: type widening on optionsAccepting MeshPubSub | HivePubSub on InMemoryLRUCacheOptions.pubsub is correct and consistent across the PR.
packages/legacy/types/src/index.ts (2)
5-6: Imports look correct for Hive PubSub integration.Using a type-only import for HivePubSub and a value import for HiveMeshPubSub is appropriate.
89-90: Publicly exporting HivePubSub type is appropriate.This exposes the new type to downstream packages without affecting runtime.
packages/plugins/live-query/src/index.ts (1)
5-11: Imports updated to support Hive: looks good.Bringing in toMeshPubSub and HivePubSub from @graphql-mesh/types aligns with the new normalization approach.
packages/legacy/utils/src/resolve-additional-resolvers.ts (1)
16-22: Type imports expanded correctly for Hive; guard usage looks consistent.Using isHivePubSub to branch subscription logic matches the new union type.
packages/cache/redis/src/index.ts (1)
5-13: Constructor type widened and normalized via toMeshPubSub: good change.Accepting HivePubSub in options and normalizing it locally keeps external API flexible while preserving internal expectations.
Also applies to: 23-26
packages/transports/rest/src/directives/process.ts (1)
11-12: Type widening for pubsub in ProcessDirectiveArgs is appropriate.This aligns the directive processing layer with the new Hive PubSub support.
Also applies to: 28-30
packages/transports/rest/src/directives/pubsubOperation.ts (2)
3-3: Imports for Hive support look correct and minimal.Bringing in toMeshPubSub and HivePubSub is the right approach to normalize at the boundary while keeping internal logic unchanged.
8-8: Good: widened globalPubsub type to accept HivePubSub.This preserves backward compatibility while allowing new Hive-based sources.
Apollo Federation Subgraph Compatibility Results
Learn more: |
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@graphql-mesh/cache-cfw-kv |
0.105.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/cache-file |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/cache-inmemory-lru |
0.8.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/cache-localforage |
0.105.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/cache-redis |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/cache-upstash-redis |
0.1.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/compose-cli |
1.4.13-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/fusion-composition |
0.8.12-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/include |
0.3.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/apollo-link |
0.106.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/cli |
0.100.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/config |
0.108.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/graphql |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/grpc |
0.108.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/json-schema |
0.109.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/mongoose |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/mysql |
0.105.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/neo4j |
0.107.6-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/odata |
0.106.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/openapi |
0.109.15-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/postgraphile |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/raml |
0.109.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/soap |
0.107.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/supergraph |
0.10.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/thrift |
0.106.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/tuql |
0.105.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/http |
0.106.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/merger-bare |
0.105.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/merger-stitching |
0.105.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/migrate-config-cli |
1.6.14-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/runtime |
0.106.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/store |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-cache |
0.105.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-encapsulate |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-extend |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-federation |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-filter-schema |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-hive |
0.104.10-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-hoist-field |
0.105.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-naming-convention |
0.105.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-prefix |
0.105.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-prune |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-rate-limit |
0.105.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-rename |
0.105.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-replace-field |
0.105.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-resolvers-composition |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-transfer-schema |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transform-type-merging |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/types |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/urql-exchange |
0.106.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/utils |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@omnigraph/json-schema |
0.109.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@omnigraph/mysql |
0.9.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@omnigraph/neo4j |
0.11.6-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@omnigraph/odata |
0.2.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@omnigraph/openapi |
0.109.15-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@omnigraph/raml |
0.109.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@omnigraph/soap |
0.107.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@omnigraph/sqlite |
0.8.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@omnigraph/thrift |
0.9.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-deduplicate-request |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-hive |
0.104.10-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-http-cache |
0.105.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-http-details-extensions |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-jit |
0.2.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-live-query |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-mock |
0.105.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-newrelic |
0.104.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-operation-field-permissions |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-operation-headers |
1.4.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-rate-limit |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-response-cache |
0.104.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-snapshot |
0.104.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-statsd |
0.104.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-grpc |
0.3.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-mysql |
0.9.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-neo4j |
0.10.6-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-odata |
0.2.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-rest |
0.9.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-soap |
0.10.9-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-sqlite |
0.9.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-thrift |
0.9.8-alpha-20250825140658-988f735d5b841746038b8f3fd125b97353823c82 |
npm ↗︎ unpkg ↗︎ |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/legacy/types/src/pubsub.ts (1)
124-127: LGTM: Type guard now safely handles undefined.The null check avoids the runtime error and satisfies TS for the “in” operator. Good fix.
packages/cache/inmemory-lru/src/index.ts (1)
22-25: Guard unsubscribe when pubsub may be undefined; fix possible runtime error.toMeshPubSub(options?.pubsub) can be undefined. Without optional chaining on the object,
pubsub.unsubscribe(subId)can throw.- const pubsub = toMeshPubSub(options?.pubsub); - const subId = pubsub?.subscribe?.('destroy', () => { - pubsub.unsubscribe(subId); + const pubsub = toMeshPubSub(options?.pubsub); + const subId = pubsub?.subscribe?.('destroy', () => { + pubsub?.unsubscribe?.(subId as number); this[DisposableSymbols.dispose](); });
🧹 Nitpick comments (6)
.changeset/@graphql-mesh_types-8716-dependencies.md (2)
5-5: Fix incorrect npm link version (alpha vs stable).The dependency string pins an alpha build (
2.0.0-alpha-86d532…) but the link points tov/2.0.0(stable). Update the link to match the actual version to avoid confusion.- - Added dependency [`@graphql-hive/[email protected]` ↗︎](https://www.npmjs.com/package/@graphql-hive/pubsub/v/2.0.0) (to `dependencies`) + - Added dependency [`@graphql-hive/[email protected]` ↗︎](https://www.npmjs.com/package/@graphql-hive/pubsub/v/2.0.0-alpha-86d532fca168f7b6e57de46d3cff2439cc6242b7) (to `dependencies`)
4-7: Fix markdown list indentation to satisfy markdownlint (MD007).Unordered list items should not be indented under the heading text.
-dependencies updates: - - Added dependency [`@graphql-hive/[email protected]` ↗︎](https://www.npmjs.com/package/@graphql-hive/pubsub/v/2.0.0) (to `dependencies`) - - Added dependency [`@repeaterjs/repeater@^3.0.6` ↗︎](https://www.npmjs.com/package/@repeaterjs/repeater/v/3.0.6) (to `dependencies`) - - Added dependency [`@whatwg-node/disposablestack@^0.0.6` ↗︎](https://www.npmjs.com/package/@whatwg-node/disposablestack/v/0.0.6) (to `dependencies`) +dependencies updates: +- Added dependency [`@graphql-hive/[email protected]` ↗︎](https://www.npmjs.com/package/@graphql-hive/pubsub/v/2.0.0-alpha-86d532fca168f7b6e57de46d3cff2439cc6242b7) (to `dependencies`) +- Added dependency [`@repeaterjs/repeater@^3.0.6` ↗︎](https://www.npmjs.com/package/@repeaterjs/repeater/v/3.0.6) (to `dependencies`) +- Added dependency [`@whatwg-node/disposablestack@^0.0.6` ↗︎](https://www.npmjs.com/package/@whatwg-node/disposablestack/v/0.0.6) (to `dependencies`)packages/legacy/types/src/pubsub.ts (4)
63-66: Avoid subId collisions by retrying on existing keys.Using a random number without collision handling can lead to rare but nasty bugs. Add a simple retry to guarantee uniqueness.
- const subId = Math.floor(Math.random() * 100_000_000); + let subId = Math.floor(Math.random() * 100_000_000); + while (this.#subs.has(subId)) { + subId = Math.floor(Math.random() * 100_000_000); + }
82-93: UseisPromisefor consistency and clearer intent.You already import
isPromise. Prefer it over checking'then' in unsubscribe.- if ('then' in unsubscribe) { - unsubscribe.then(unsub => { + if (isPromise(unsubscribe)) { + unsubscribe.then(unsub => { const unsubbed$ = unsub(); if (isPromise(unsubbed$)) { unsubbed$.catch(err => { console.error(`Failed to unsubscribe from ${subId}`, err); }); } }); } else { unsubscribe(); }
129-152: Typo in variable name:meshForHibePubSub→meshForHivePubSub.Minor but worth fixing for readability and grep-ability.
-const meshForHibePubSub = new WeakMap<HivePubSub, MeshPubSub>(); +const meshForHivePubSub = new WeakMap<HivePubSub, MeshPubSub>(); @@ - if (isHivePubSub(pubsub)) { - let hivePubsub = meshForHibePubSub.get(pubsub); + if (isHivePubSub(pubsub)) { + let hivePubsub = meshForHivePubSub.get(pubsub); if (hivePubsub) { return hivePubsub; } hivePubsub = MeshFromHivePubSub.from(pubsub); - meshForHibePubSub.set(pubsub, hivePubsub); + meshForHivePubSub.set(pubsub, hivePubsub); return hivePubsub; }
111-117: Optional: annotate return types for disposal methods.Adding explicit return types improves TS strictness and DX.
- public dispose() { + public dispose(): MaybePromise<void> { return this.#pubsub.dispose(); } - [DisposableSymbols.asyncDispose]() { + [DisposableSymbols.asyncDispose](): MaybePromise<void> { return this.#pubsub.dispose(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
.changeset/@graphql-mesh_types-8716-dependencies.md(1 hunks)packages/cache/inmemory-lru/src/index.ts(1 hunks)packages/legacy/types/package.json(1 hunks)packages/legacy/types/src/index.ts(1 hunks)packages/legacy/types/src/pubsub.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/legacy/types/package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-31T09:44:11.443Z
Learnt from: ardatan
PR: ardatan/graphql-mesh#8350
File: packages/cache/upstash-redis/src/index.ts:26-34
Timestamp: 2025-01-31T09:44:11.443Z
Learning: In GraphQL Mesh's KeyValueCache implementation, the `options.ttl` parameter in the `set` method is specified in seconds, which needs to be converted to milliseconds when using Redis's `px` option.
Applied to files:
packages/cache/inmemory-lru/src/index.ts
🧬 Code Graph Analysis (2)
packages/cache/inmemory-lru/src/index.ts (1)
packages/legacy/types/src/pubsub.ts (3)
MeshPubSub(14-24)HivePubSub(6-6)toMeshPubSub(144-155)
packages/legacy/types/src/pubsub.ts (2)
packages/legacy/utils/src/index.ts (1)
MaybePromise(13-13)packages/cache/inmemory-lru/src/index.ts (1)
DisposableSymbols(68-74)
🪛 LanguageTool
.changeset/@graphql-mesh_types-8716-dependencies.md
[grammar] ~4-~4: There might be a mistake here.
Context: .../types": patch --- dependencies updates: - Added dependency [`@graphql-hive/pubsub@...
(QB_NEW_EN)
[grammar] ~5-~5: There might be a mistake here.
Context: ...hive/pubsub/v/2.0.0) (to dependencies) - Added dependency [`@repeaterjs/repeater@...
(QB_NEW_EN)
[grammar] ~6-~6: There might be a mistake here.
Context: ...js/repeater/v/3.0.6) (to dependencies) - Added dependency [`@whatwg-node/disposab...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ...osablestack/v/0.0.6) (to dependencies)
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
.changeset/@graphql-mesh_types-8716-dependencies.md
5-5: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
6-6: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
7-7: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
⏰ 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). (13)
- GitHub Check: apollo-federation-compatibility
- GitHub Check: check
- GitHub Check: unit / node 20
- GitHub Check: e2e / node v20
- GitHub Check: integration / node 18
- GitHub Check: unit / node 18
- GitHub Check: integration / node 22
- GitHub Check: e2e / node v22
- GitHub Check: unit / node 22
- GitHub Check: e2e / node v18
- GitHub Check: integration / node 20
- GitHub Check: deployment
- GitHub Check: release / snapshot
🔇 Additional comments (2)
packages/cache/inmemory-lru/src/index.ts (1)
33-40: TTL conversion is correct (seconds → ms).Matches prior guidance for TTL handling across caches.
packages/legacy/types/src/index.ts (1)
17-20: Consolidation via pubsub re-export looks good.Importing MeshPubSub from the dedicated pubsub module and re-exporting the bridge keeps the public surface coherent while enabling Hive support behind the scenes.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/legacy/types/src/pubsub.ts (1)
110-115: LGTM: Iterator-to-array bug fixed as suggested earlier.The conversion to
Array.from(this.#subs.values(), ...)avoids calling array methods on an iterator. Thanks for addressing.
🧹 Nitpick comments (7)
e2e/openapi-javascript-wiki/mesh.config.ts (1)
25-29: Nit: Prefer canonical 'User-Agent' casing and double-check wildcard scope.Same minor suggestion as in the other file:
- headers: [['user-agent', 'hive-gateway/e2e']], + headers: [['User-Agent', 'hive-gateway/e2e']],If the intent is to target all REST transports for this e2e, "*.rest" is fine; otherwise narrow it to the specific transport key if the gateway supports that granularity.
You can reuse the audit script from the other comment to confirm scope and consistency across e2e configs.
packages/legacy/types/src/pubsub.ts (6)
26-33: Fix TSDoc links and wording for accuracy.Links like
{@link PubSub Hive PubSub interface}won't resolve. Link to the actual exported types/symbols./** - * Converts the {@link PubSub Hive PubSub interface} to the legacy {@link MeshPubSub} - * Please avoid using this class directly because it will be completely removed in - * the future, instead migrate your project to use the {@link PubSub new interface}. + * Converts a Hive PubSub ({@link HivePubSub}) to the legacy {@link MeshPubSub}. + * Please avoid using this class directly because it will be completely removed in + * the future; instead migrate your project to use the new Hive PubSub interface. * - * @deprecated This class is deprecated and will be removed in the future. Implement and use the new {@link PubSub Hive PubSub interface} instead. + * @deprecated This class is deprecated and will be removed in the future. Implement and use {@link HivePubSub} instead of {@link MeshPubSub}. */
47-50: Remove redundant overload signature.You have an overload for
static from(pubsub: undefined | HivePubSub)immediately followed by the implementation with the exact same signature. Keep the specific overloads (undefined, HivePubSub) and a single broad implementation.- static from(pubsub: undefined | HivePubSub): undefined | MeshFromHivePubSub; static from(pubsub: undefined | HivePubSub): undefined | MeshFromHivePubSub { if (!pubsub) return undefined; return new MeshFromHivePubSub(pubsub); }
125-132: Consider clearing internal state on dispose to aid GC.Clearing
#subsreleases references to callbacks and trigger names even if the underlying pubsub handles cleanup.public dispose() { - return this.#pubsub.dispose(); + this.#subs.clear(); + return this.#pubsub.dispose(); } [DisposableSymbols.asyncDispose]() { - return this.#pubsub.dispose(); + this.#subs.clear(); + return this.#pubsub.dispose(); }
134-141: Optionally harden the type guard to reduce false-positives.Checking only for missing
asyncIteratormight classify any non-Mesh-like object as Hive. Add existence checks forpublishandsubscribe.export function isHivePubSub(pubsub: undefined | MeshPubSub | HivePubSub): pubsub is HivePubSub { - // HivePubSub does not have asyncIterator method. this only applies for @graphql-hive/pubsub v2+ - return pubsub != null && !('asyncIterator' in pubsub); + // HivePubSub does not have asyncIterator (applies for @graphql-hive/pubsub v2+) + return ( + pubsub != null && + typeof (pubsub as any).publish === 'function' && + typeof (pubsub as any).subscribe === 'function' && + !('asyncIterator' in (pubsub as any)) + ); }
143-166: Typo: rename meshForHibePubSub → meshForHivePubSub and improve variable naming.Minor readability improvements; also rename local
hivePubsubtomeshPubsubto reflect its type.-const meshForHibePubSub = new WeakMap<HivePubSub, MeshPubSub>(); +const meshForHivePubSub = new WeakMap<HivePubSub, MeshPubSub>(); @@ if (isHivePubSub(pubsub)) { - let hivePubsub = meshForHibePubSub.get(pubsub); - if (hivePubsub) { - return hivePubsub; + let meshPubsub = meshForHivePubSub.get(pubsub); + if (meshPubsub) { + return meshPubsub; } - hivePubsub = MeshFromHivePubSub.from(pubsub); - meshForHibePubSub.set(pubsub, hivePubsub); - return hivePubsub; + meshPubsub = MeshFromHivePubSub.from(pubsub); + meshForHivePubSub.set(pubsub, meshPubsub); + return meshPubsub; }
153-158: Overload set can be simplified.The current set is verbose. Keeping just these is sufficient and clearer:
- toMeshPubSub(pubsub: undefined): undefined
- toMeshPubSub(pubsub: HivePubSub | MeshPubSub): MeshPubSub
If you prefer, I can draft a concise overload set.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
e2e/openapi-additional-resolvers/mesh.config.ts(1 hunks)e2e/openapi-javascript-wiki/mesh.config.ts(1 hunks)packages/legacy/types/src/pubsub.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/legacy/types/src/pubsub.ts (2)
packages/legacy/utils/src/index.ts (1)
MaybePromise(13-13)packages/cache/inmemory-lru/src/index.ts (1)
DisposableSymbols(68-74)
⏰ 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). (12)
- GitHub Check: unit / node 22
- GitHub Check: unit / node 18
- GitHub Check: unit / node 20
- GitHub Check: check
- GitHub Check: e2e / node v22
- GitHub Check: e2e / node v18
- GitHub Check: integration / node 22
- GitHub Check: integration / node 20
- GitHub Check: e2e / node v20
- GitHub Check: integration / node 18
- GitHub Check: release / snapshot
- GitHub Check: deployment
🔇 Additional comments (4)
e2e/openapi-additional-resolvers/mesh.config.ts (2)
23-27: LGTM: Adding a REST-specific user-agent via transportEntries is appropriate for e2e traffic shaping.This makes outbound REST calls identifiable and is consistent with the parallel update in the other e2e config.
23-27: Per-test scope confirmed; optional header-casing nit
- Both
e2e/openapi-javascript-wiki/mesh.config.tsande2e/openapi-additional-resolvers/mesh.config.tsindependently define their own
transportEntries['*.rest'], so the wildcard only applies within each test’s gateway config.- If you prefer conventional header casing, you can update:
- headers: [['user-agent', 'hive-gateway/e2e']], + headers: [['User-Agent', 'hive-gateway/e2e']],Otherwise, everything looks intended.
e2e/openapi-javascript-wiki/mesh.config.ts (1)
25-29: LGTM: Consistent user-agent header for REST transport in e2e.Matches the additional-resolvers config and should help identify/shape outbound REST traffic during tests.
packages/legacy/types/src/pubsub.ts (1)
117-123: LGTM: Clean async iterator lifecycle via Repeater.The subscription is torn down on stop, matching expectations for AsyncIterable sources.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
packages/legacy/types/src/pubsub.ts (4)
55-62: Catch synchronous publish errors to avoid process crashes
publishonly guards async rejections; a sync throw from the underlyingHivePubSub.publishwould currently bubble and crash the process. Wrap in try/catch while keeping the async guard.publish<THook extends HookName>(triggerName: THook, payload: AllHooks[THook]): void { - const publishing = this.#pubsub.publish(triggerName, payload); - if (isPromise(publishing)) { - publishing.catch(err => { - console.error(`Failed to publish to ${triggerName}`, err); - }); - } + try { + const publishing = this.#pubsub.publish(triggerName, payload); + if (isPromise(publishing)) { + publishing.catch(err => { + console.error(`Failed to publish to ${triggerName}`, err); + }); + } + } catch (err) { + console.error(`Failed to publish to ${triggerName}`, err); + } }
89-96: Harden unsubscribe against synchronous errors thrown by handlersIf
unsub()throws synchronously, it will crash the caller. Catch sync errors in both branches.unsubscribe .then(unsub => { - const unsubbed = unsub(); - if (isPromise(unsubbed)) { - unsubbed.catch(err => { - console.error(`Failed to finish unsubscribe from ${subId}`, err); - }); - } + try { + const unsubbed = unsub(); + if (isPromise(unsubbed)) { + unsubbed.catch(err => { + console.error(`Failed to finish unsubscribe from ${subId}`, err); + }); + } + } catch (err) { + console.error(`Failed to finish unsubscribe from ${subId}`, err); + } }) .catch(err => { console.error(`Failed to start unsubscribe from ${subId}`, err); }); } else { - const unsubbed = unsubscribe(); - if (isPromise(unsubbed)) { - unsubbed.catch(err => { - console.error(`Failed to finish unsubscribe from ${subId}`, err); - }); - } + try { + const unsubbed = unsubscribe(); + if (isPromise(unsubbed)) { + unsubbed.catch(err => { + console.error(`Failed to finish unsubscribe from ${subId}`, err); + }); + } + } catch (err) { + console.error(`Failed to finish unsubscribe from ${subId}`, err); + } }Also applies to: 101-107
110-117: LGTM: iterator-to-array fix avoids.maponMap.prototype.values()Using
Array.from(this.#subs.values(), ...)correctly converts the iterator and maps in one pass. This resolves the runtime crash risk noted earlier.
64-70: Fix interface mismatch (missing options param) and eliminate subId collision risk
MeshFromHivePubSub.subscribedoes not accept the optionaloptionsargument required byMeshPubSub, so the class does not conform to its own interface. This will fail type-checking.- Use a collision-free
subIdgenerator;Math.randomcan theoretically collide.subscribe<THook extends HookName>( triggerName: THook, onMessage: (data: AllHooks[THook]) => void, - ): number { - const subId = Math.floor(Math.random() * 100_000_000); + _options?: any, + ): number { + // Generate a collision-free subscription id + let subId: number; + do { + subId = Math.floor(Math.random() * 100_000_000); + } while (this.#subs.has(subId)); const unsub = this.#pubsub.subscribe(triggerName, onMessage); this.#subs.set(subId, { triggerName, unsubscribe: unsub });
🧹 Nitpick comments (4)
packages/legacy/types/src/pubsub.ts (4)
140-143: Tighten theisHivePubSubtype guard to reduce false positivesRelying solely on the absence of
asyncIteratorcan misclassify arbitrary objects. Check for the presence of expected function members too.-export function isHivePubSub(pubsub: undefined | MeshPubSub | HivePubSub): pubsub is HivePubSub { - // HivePubSub does not have asyncIterator method. this only applies for @graphql-hive/pubsub v2+ - return pubsub != null && !('asyncIterator' in pubsub); -} +export function isHivePubSub(pubsub: undefined | MeshPubSub | HivePubSub): pubsub is HivePubSub { + // HivePubSub v2+ lacks asyncIterator; also ensure publish/subscribe exist to avoid false positives + return ( + pubsub != null && + typeof (pubsub as any).publish === 'function' && + typeof (pubsub as any).subscribe === 'function' && + !('asyncIterator' in (pubsub as any)) + ); +}
145-171: Nit: fix variable typo (Hibe➜Hive) and apply rename at usagesPurely cosmetic but improves readability and grep-ability.
-const meshForHibePubSub = new WeakMap<HivePubSub, MeshPubSub>(); +const meshForHivePubSub = new WeakMap<HivePubSub, MeshPubSub>(); export function toMeshPubSub(pubsub?: MeshPubSub | HivePubSub | undefined): MeshPubSub | undefined { if (isHivePubSub(pubsub)) { - let hivePubsub = meshForHibePubSub.get(pubsub); + let hivePubsub = meshForHivePubSub.get(pubsub); if (hivePubsub) { return hivePubsub; } hivePubsub = MeshFromHivePubSub.from(pubsub); - meshForHibePubSub.set(pubsub, hivePubsub); + meshForHivePubSub.set(pubsub, hivePubsub); return hivePubsub; } return pubsub; }
49-49: Remove redundant overload signatureThe overload on Line 49 duplicates the implementation signature on Line 50. Safe to drop to reduce noise.
- static from(pubsub: undefined | HivePubSub): undefined | MeshFromHivePubSub;
127-133: Add explicit return types to disposal methodsMinor clarity: annotate disposal methods with
MaybePromise<void>(matches underlyingHivePubSub.dispose()), aiding consumers and tooling.- public dispose() { + public dispose(): MaybePromise<void> { return this.#pubsub.dispose(); } - [DisposableSymbols.asyncDispose]() { + [DisposableSymbols.asyncDispose](): MaybePromise<void> { return this.#pubsub.dispose(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/legacy/types/src/pubsub.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/legacy/types/src/pubsub.ts (2)
packages/legacy/utils/src/index.ts (1)
MaybePromise(13-13)packages/cache/inmemory-lru/src/index.ts (1)
DisposableSymbols(68-74)
⏰ 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). (13)
- GitHub Check: release / snapshot
- GitHub Check: deployment
- GitHub Check: integration / node 22
- GitHub Check: integration / node 18
- GitHub Check: apollo-federation-compatibility
- GitHub Check: integration / node 20
- GitHub Check: e2e / node v20
- GitHub Check: e2e / node v22
- GitHub Check: unit / node 22
- GitHub Check: unit / node 18
- GitHub Check: unit / node 20
- GitHub Check: e2e / node v18
- GitHub Check: check
enisdenjo
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.changeset/@graphql-mesh_types-8716-dependencies.md (2)
4-4: Polish wording: “Dependency updates” (singular “Dependency”) reads better.Minor copy edit for clarity/consistency with common Changeset phrasing.
Apply this diff:
-dependencies updates: +Dependency updates:
5-7: Fix list indentation to satisfy markdownlint (MD007).Top‑level list items shouldn’t be indented here.
Apply this diff:
- - Added dependency [`@graphql-hive/pubsub@next` ↗︎](https://www.npmjs.com/package/@graphql-hive/pubsub/v/next) (to `dependencies`) - - Added dependency [`@repeaterjs/repeater@^3.0.6` ↗︎](https://www.npmjs.com/package/@repeaterjs/repeater/v/3.0.6) (to `dependencies`) - - Added dependency [`@whatwg-node/disposablestack@^0.0.6` ↗︎](https://www.npmjs.com/package/@whatwg-node/disposablestack/v/0.0.6) (to `dependencies`) +- Added dependency [`@graphql-hive/pubsub@next` ↗︎](https://www.npmjs.com/package/@graphql-hive/pubsub/v/next) (to `dependencies`) +- Added dependency [`@repeaterjs/repeater@^3.0.6` ↗︎](https://www.npmjs.com/package/@repeaterjs/repeater/v/3.0.6) (to `dependencies`) +- Added dependency [`@whatwg-node/disposablestack@^0.0.6` ↗︎](https://www.npmjs.com/package/@whatwg-node/disposablestack/v/0.0.6) (to `dependencies`)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
.changeset/@graphql-mesh_types-8716-dependencies.md(1 hunks).changeset/cute-ducks-brush.md(1 hunks).changeset/hungry-suns-appear.md(1 hunks)packages/legacy/types/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .changeset/cute-ducks-brush.md
- .changeset/hungry-suns-appear.md
- packages/legacy/types/package.json
🧰 Additional context used
🪛 LanguageTool
.changeset/@graphql-mesh_types-8716-dependencies.md
[grammar] ~4-~4: There might be a mistake here.
Context: .../types": patch --- dependencies updates: - Added dependency [`@graphql-hive/pubsub@...
(QB_NEW_EN)
[grammar] ~5-~5: There might be a mistake here.
Context: ...-hive/pubsub/v/next) (to dependencies) - Added dependency [`@repeaterjs/repeater@...
(QB_NEW_EN)
[grammar] ~6-~6: There might be a mistake here.
Context: ...js/repeater/v/3.0.6) (to dependencies) - Added dependency [`@whatwg-node/disposab...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ...osablestack/v/0.0.6) (to dependencies)
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
.changeset/@graphql-mesh_types-8716-dependencies.md
5-5: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
6-6: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
7-7: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
⏰ 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). (12)
- GitHub Check: integration / node 22
- GitHub Check: e2e / node v20
- GitHub Check: e2e / node v22
- GitHub Check: e2e / node v18
- GitHub Check: integration / node 18
- GitHub Check: unit / node 22
- GitHub Check: unit / node 18
- GitHub Check: integration / node 20
- GitHub Check: unit / node 20
- GitHub Check: check
- GitHub Check: deployment
- GitHub Check: release / snapshot
🔇 Additional comments (2)
.changeset/@graphql-mesh_types-8716-dependencies.md (2)
1-3: Front‑matter structure looks correct for a Changeset.Package name, bump type, and delimiters are in the expected format.
6-7: Dependencies correctly classifiedVerified that both
@repeaterjs/repeaterand@whatwg-node/disposablestackare imported as regular (non-type-only) modules inpackages/legacy/types/src/pubsub.ts, indicating runtime usage. Keeping them underdependenciesis therefore appropriate.
Ref GW-448
See graphql-hive/gateway#1395 for more info.
Only components on which the Hive Gateway depends support both the new
HivePubSuband the legacyMeshPubSub. Added two more utilities:isHivePubSubandtoMeshPubSubfor easier usage and backwards compatibility.TODO
@graphql-hive/pubsubonce Hive Gateway v2 is released