-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Allow QueryManager to intercept hook functionality #11617
Changes from 13 commits
c518f31
6615778
6d573c5
45fc3b3
d7934e0
ce8e6e7
bb2c718
c153a4e
7497c9c
94aa747
5519a64
c58802e
a114bb0
a274f96
57a6c75
d019058
682aa45
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@apollo/client": patch | ||
--- | ||
|
||
Allow Apollo Client instance to intercept hook functionality |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
{ | ||
"dist/apollo-client.min.cjs": 39075, | ||
"dist/apollo-client.min.cjs": 39283, | ||
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32584 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
import type { | ||
useQuery, | ||
useSuspenseQuery, | ||
useBackgroundQuery, | ||
useReadQuery, | ||
useFragment, | ||
} from "../index.js"; | ||
import type { QueryManager } from "../../../core/QueryManager.js"; | ||
import type { ApolloClient } from "../../../core/ApolloClient.js"; | ||
import type { ObservableQuery } from "../../../core/ObservableQuery.js"; | ||
|
||
const wrapperSymbol = Symbol.for("apollo.hook.wrappers"); | ||
|
||
interface WrappableHooks { | ||
useQuery: typeof useQuery; | ||
useSuspenseQuery: typeof useSuspenseQuery; | ||
useBackgroundQuery: typeof useBackgroundQuery; | ||
useReadQuery: typeof useReadQuery; | ||
useFragment: typeof useFragment; | ||
} | ||
|
||
/** | ||
* @internal | ||
* Can be used to correctly type the [Symbol.for("apollo.hook.wrappers")] of | ||
* a class that extends `ApolloClient`, to override/wrap hook functionality. | ||
*/ | ||
export type HookWrappers = { | ||
[K in keyof WrappableHooks]?: ( | ||
originalHook: WrappableHooks[K] | ||
) => WrappableHooks[K]; | ||
}; | ||
|
||
interface QueryManagerWithWrappers<T> extends QueryManager<T> { | ||
[wrapperSymbol]?: HookWrappers; | ||
} | ||
|
||
/** | ||
* @internal | ||
* | ||
* Makes an Apollo Client hook "wrappable". | ||
* That means that the Apollo Client instance can expose a "wrapper" that will be | ||
* used to wrap the original hook implementation with additional logic. | ||
* @example | ||
* ```tsx | ||
* // this is already done in `@apollo/client` for all wrappable hooks (see `WrappableHooks`) | ||
* const wrappedUseQuery = makeHookWrappable('useQuery', useQuery, (_, options) => options.client); | ||
* | ||
* // although for tree-shaking purposes, in reality it looks more like | ||
* function useQuery() { | ||
* useQuery = makeHookWrappable('useQuery', (_, options) => options.client, _useQuery); | ||
* return useQuery.apply(null, arguments as any); | ||
* } | ||
* function _useQuery() { | ||
* // original implementation | ||
* } | ||
* | ||
* // this is what a library like `@apollo/client-react-streaming` would do | ||
* class ApolloClientWithStreaming extends ApolloClient { | ||
* constructor(options) { | ||
* super(options); | ||
* this.queryManager[Symbol.for("apollo.hook.wrappers")] = { | ||
* useQuery: (original) => (query, options) => { | ||
* console.log("useQuery was called with options", options); | ||
* return original(query, options); | ||
* } | ||
* } | ||
* } | ||
* } | ||
* | ||
* // this will now log the options and then call the original `useQuery` | ||
* const client = new ApolloClientWithStreaming({ ... }); | ||
* wrappedUseQuery(query, { client }); | ||
* ``` | ||
*/ | ||
/*#__NO_SIDE_EFFECTS__*/ | ||
export function makeHookWrappable<Name extends keyof WrappableHooks>( | ||
hookName: Name, | ||
getClientFromOptions: ( | ||
...args: Parameters<WrappableHooks[Name]> | ||
) => ObservableQuery<any> | ApolloClient<any>, | ||
useHook: WrappableHooks[Name] | ||
): WrappableHooks[Name] { | ||
return function (this: any) { | ||
const args = arguments as unknown as Parameters<WrappableHooks[Name]>; | ||
const queryManager = ( | ||
getClientFromOptions.apply(this, args) as unknown as { | ||
// both `ApolloClient` and `ObservableQuery` have a `queryManager` property | ||
// but they're both `private`, so we have to cast around for a bit here. | ||
queryManager: QueryManagerWithWrappers<any>; | ||
} | ||
)["queryManager"]; | ||
const wrappers = queryManager && queryManager[wrapperSymbol]; | ||
const wrapper = wrappers && wrappers[hookName]; | ||
const wrappedHook: WrappableHooks[Name] = | ||
wrapper ? wrapper(useHook) : useHook; | ||
return (wrappedHook as any).apply(this, args); | ||
} as any; | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -15,7 +15,7 @@ import { | |||||||||||||||||||||||||||||||||
} from "../internal/index.js"; | ||||||||||||||||||||||||||||||||||
import type { CacheKey, QueryReference } from "../internal/index.js"; | ||||||||||||||||||||||||||||||||||
import type { BackgroundQueryHookOptions, NoInfer } from "../types/types.js"; | ||||||||||||||||||||||||||||||||||
import { __use } from "./internal/index.js"; | ||||||||||||||||||||||||||||||||||
import { __use, makeHookWrappable } from "./internal/index.js"; | ||||||||||||||||||||||||||||||||||
import { useWatchQueryOptions } from "./useSuspenseQuery.js"; | ||||||||||||||||||||||||||||||||||
import type { FetchMoreFunction, RefetchFunction } from "./useSuspenseQuery.js"; | ||||||||||||||||||||||||||||||||||
import { canonicalStringify } from "../../cache/index.js"; | ||||||||||||||||||||||||||||||||||
|
@@ -170,7 +170,19 @@ export function useBackgroundQuery< | |||||||||||||||||||||||||||||||||
UseBackgroundQueryResult<TData, TVariables>, | ||||||||||||||||||||||||||||||||||
]; | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
export function useBackgroundQuery< | ||||||||||||||||||||||||||||||||||
export function useBackgroundQuery() { | ||||||||||||||||||||||||||||||||||
// @ts-expect-error Cannot assign to 'useBackgroundQuery' because it is a function.ts(2630) | ||||||||||||||||||||||||||||||||||
useBackgroundQuery = makeHookWrappable( | ||||||||||||||||||||||||||||||||||
"useBackgroundQuery", | ||||||||||||||||||||||||||||||||||
(_, options) => | ||||||||||||||||||||||||||||||||||
useApolloClient(typeof options === "object" ? options.client : undefined), | ||||||||||||||||||||||||||||||||||
_useBackgroundQuery as any | ||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
return useBackgroundQuery.apply(null, arguments as any); | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps I'm missing something obvious or its been a long travel day, but couldn't you just set this to any variable name and call it? Any reason you need to override the original function?
Suggested change
or if if you wanted to do it all inline return makeHookWrappable(
"useBackgroundQuery",
(_, options) =>
useApolloClient(typeof options === "object" ? options.client : undefined),
_useBackgroundQuery as any
).apply(null, arguments as any); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is based on a babel pattern: function _extends() {
_extends = Object.assign
? Object.assign.bind()
: function (target) {
for (var i = 1; i < arguments.length; i++) {
var source = arguments[i];
for (var key in source) {
if (Object.prototype.hasOwnProperty.call(source, key)) {
target[key] = source[key];
}
}
}
return target;
};
return _extends.apply(this, arguments);
}
var a = _extends({}, b); Essentially, it's a lazy override pattern, so But, one sleep later, I agree with you that this can be simplified - we are not on a hot path here (something is seriously wrong if we ever are), so we might as well optimize a bit more for readability and amount of code. I still prefer the first approach we had, overriding the hooks after creation, as that way we didn't have to touch their contents. But as we don't have that option, I believe what I've ended up doing now should be one of the simpler possible solutions. Can you please take another look? |
||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
function _useBackgroundQuery< | ||||||||||||||||||||||||||||||||||
TData = unknown, | ||||||||||||||||||||||||||||||||||
TVariables extends OperationVariables = OperationVariables, | ||||||||||||||||||||||||||||||||||
>( | ||||||||||||||||||||||||||||||||||
|
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.
We don't strictly need this here anymore in this specific case, but it would be good to have this in our bundling config anyways, so I'll leave it here.