Skip to content

Simplify and stabilise AbortSignal.any ponyfill #593

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

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@graphql-hive/gateway-abort-signal-any': patch
---

dependencies updates:

- Removed dependency [`@graphql-tools/utils@^10.7.0` ↗︎](https://www.npmjs.com/package/@graphql-tools/utils/v/10.7.0) (from `dependencies`)
- Removed dependency [`tslib@^2.8.1` ↗︎](https://www.npmjs.com/package/tslib/v/2.8.1) (from `dependencies`)
- Removed dependency [`graphql@^15.0.0 || ^16.9.0 || ^17.0.0` ↗︎](https://www.npmjs.com/package/graphql/v/15.0.0) (from `peerDependencies`)
5 changes: 5 additions & 0 deletions .changeset/famous-mangos-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-hive/gateway-abort-signal-any': major
---

`abortSignalAny` uses `AbortSignal.any` if available and falls back to `AbortController` for ponyfilling
7 changes: 7 additions & 0 deletions .changeset/little-plums-perform.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@graphql-hive/gateway-abort-signal-any': major
---

`abortSignalAny` returns the native `AbortSignal`

Instead of the custom `AbortSignalFromAny`. There is no need for the complication of adding signals to existing sets, chaining signals enough as well as simpler.
5 changes: 5 additions & 0 deletions .changeset/long-beds-dream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-hive/gateway-abort-signal-any': major
---

`AbortSignal.any` ponyfill under `abortSignalAny` is now stable
5 changes: 5 additions & 0 deletions .changeset/real-seals-taste.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-hive/gateway-runtime': patch
---

Use improved AbortSignal.any ponyfill fixing potential memory leaks
11 changes: 0 additions & 11 deletions packages/abort-signal-any/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,8 @@
"build": "pkgroll --clean-dist",
"prepack": "yarn build"
},
"peerDependencies": {
"graphql": "^15.0.0 || ^16.9.0 || ^17.0.0"
},
"dependencies": {
"@graphql-tools/utils": "^10.7.0",
"tslib": "^2.8.1"
},
"devDependencies": {
"graphql": "^16.9.0",
"pkgroll": "2.6.1"
},
"publishConfig": {
"access": "public"
},
"sideEffects": false
}
77 changes: 20 additions & 57 deletions packages/abort-signal-any/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,65 +1,28 @@
import { registerAbortSignalListener } from '@graphql-tools/utils';
export function abortSignalAny(iterable: Iterable<AbortSignal>) {
const signals = Array.from(iterable);

export type AbortSignalFromAny = AbortSignal & {
signals: Set<AbortSignal>;
addSignals(signals: Iterable<AbortSignal>): void;
};
const aborted = signals.find((s) => s.aborted);
if (aborted) {
// some signal is already aborted, immediately abort
return aborted;
}

export function isAbortSignalFromAny(
signal?: AbortSignal | null,
): signal is AbortSignalFromAny {
return signal != null && 'signals' in signal && 'addSignals' in signal;
}
// TODO: we cant use the native abortsignal.any because it doesnt work with signals coming from whatwg-node (ServerAdapterRequestAbortSignal)
// if ('any' in AbortSignal) {
// return AbortSignal.any(signals);
// }

export function abortSignalAny(givenSignals: Iterable<AbortSignal>) {
const signals = new Set<AbortSignal>();
let singleSignal: AbortSignal | undefined;
for (const signal of givenSignals) {
if (isAbortSignalFromAny(signal)) {
for (const childSignal of signal.signals) {
singleSignal = childSignal;
signals.add(childSignal);
}
} else {
singleSignal = signal;
signals.add(signal);
}
}
if (signals.size < 2) {
return singleSignal;
}
if (signals.size === 0) {
return undefined;
}
// otherwise ready a controller and listen for abort signals
const ctrl = new AbortController();
function onAbort(this: AbortSignal, ev?: Event) {
const signal = this || (ev?.target as AbortSignal);
ctrl.abort(signal?.reason);
function abort(event: Event) {
ctrl.abort((event.target as AbortSignal).reason);
// do cleanup
for (const signal of signals) {
signal.removeEventListener('abort', abort);
}
}
for (const signal of signals) {
registerAbortSignalListener(signal, onAbort);
signal.addEventListener('abort', abort);
}
Object.defineProperties(ctrl.signal, {
signals: { value: signals },
addSignals: {
value(newSignals: Iterable<AbortSignal>) {
for (const signal of newSignals) {
if (isAbortSignalFromAny(signal)) {
for (const childSignal of signal.signals) {
if (!signals.has(childSignal)) {
signals.add(childSignal);
registerAbortSignalListener(childSignal, onAbort);
}
}
} else {
if (!signals.has(signal)) {
signals.add(signal);
registerAbortSignalListener(signal, onAbort);
}
}
}
},
},
});
return ctrl.signal as AbortSignalFromAny;
return ctrl.signal;
}
25 changes: 7 additions & 18 deletions packages/runtime/src/plugins/useUpstreamCancel.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import {
abortSignalAny,
isAbortSignalFromAny,
} from '@graphql-hive/gateway-abort-signal-any';
import { abortSignalAny } from '@graphql-hive/gateway-abort-signal-any';
import { GraphQLResolveInfo } from '@graphql-tools/utils';
import type { GatewayPlugin } from '../types';

Expand All @@ -21,14 +18,10 @@ export function useUpstreamCancel(): GatewayPlugin {
if (signalInInfo) {
signals.push(signalInInfo);
}
if (isAbortSignalFromAny(options.signal)) {
options.signal.addSignals(signals);
} else {
if (options.signal) {
signals.push(options.signal);
}
options.signal = abortSignalAny(signals);
if (options.signal) {
signals.push(options.signal);
}
options.signal = abortSignalAny(signals);
},
onSubgraphExecute({ executionRequest }) {
const signals: AbortSignal[] = [];
Expand All @@ -38,14 +31,10 @@ export function useUpstreamCancel(): GatewayPlugin {
if (executionRequest.context?.request?.signal) {
signals.push(executionRequest.context.request.signal);
}
if (isAbortSignalFromAny(executionRequest.signal)) {
executionRequest.signal.addSignals(signals);
} else {
if (executionRequest.signal) {
signals.push(executionRequest.signal);
}
executionRequest.signal = abortSignalAny(signals);
if (executionRequest.signal) {
signals.push(executionRequest.signal);
}
executionRequest.signal = abortSignalAny(signals);
},
};
}
5 changes: 0 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3194,12 +3194,7 @@ __metadata:
version: 0.0.0-use.local
resolution: "@graphql-hive/gateway-abort-signal-any@workspace:packages/abort-signal-any"
dependencies:
"@graphql-tools/utils": "npm:^10.7.0"
graphql: "npm:^16.9.0"
pkgroll: "npm:2.6.1"
tslib: "npm:^2.8.1"
peerDependencies:
graphql: ^15.0.0 || ^16.9.0 || ^17.0.0
languageName: unknown
linkType: soft

Expand Down
Loading