-
Notifications
You must be signed in to change notification settings - Fork 36
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
0.9.0 (7) - Changes to Data Transport #223
Conversation
42ea24a
to
81e2d4e
Compare
I think this is ready for review now :) |
const streamObservable = new Observable< | ||
Exclude<QueryEvent, { type: "started" }> | ||
>((subscriber) => { | ||
const { markResult, markError, markReady } = queryInfo; | ||
queryInfo.markResult = function (result: FetchResult<any>) { | ||
subscriber.next({ | ||
type: "data", | ||
id, | ||
result: result, | ||
}); | ||
return markResult.apply(queryInfo, arguments as any); | ||
}; | ||
queryInfo.markError = function () { | ||
subscriber.next({ | ||
type: "error", | ||
id, | ||
}); | ||
subscriber.complete(); | ||
return markError.apply(queryInfo, arguments as any); | ||
}; | ||
queryInfo.markReady = function () { | ||
subscriber.next({ | ||
type: "complete", | ||
id, | ||
}); | ||
subscriber.complete(); | ||
return markReady.apply(queryInfo, arguments as any); | ||
}; | ||
}); |
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.
If someone has a better idea how to tap into link data events instead of wrapping queryInfo
functions, I'm all ears. This is the best I could come up with.
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.
I got nothin'
Job #14: Bundle Size — 1MiB (+0.76%).Warning Bundle contains 1 duplicate package – View duplicate packages Warning Bundle introduced one new package: react-error-boundary – View changed packages Bundle metrics
Bundle size by type
View job #14 report View pr/adjust-datatransport branch activity View project dashboard |
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.
Had one extremely small nit but this is great! I like the refactoring into events.
packages/client-react-streaming/src/DataTransportAbstraction/WrappedApolloClient.tsx
Outdated
Show resolved
Hide resolved
const streamObservable = new Observable< | ||
Exclude<QueryEvent, { type: "started" }> | ||
>((subscriber) => { | ||
const { markResult, markError, markReady } = queryInfo; | ||
queryInfo.markResult = function (result: FetchResult<any>) { | ||
subscriber.next({ | ||
type: "data", | ||
id, | ||
result: result, | ||
}); | ||
return markResult.apply(queryInfo, arguments as any); | ||
}; | ||
queryInfo.markError = function () { | ||
subscriber.next({ | ||
type: "error", | ||
id, | ||
}); | ||
subscriber.complete(); | ||
return markError.apply(queryInfo, arguments as any); | ||
}; | ||
queryInfo.markReady = function () { | ||
subscriber.next({ | ||
type: "complete", | ||
id, | ||
}); | ||
subscriber.complete(); | ||
return markReady.apply(queryInfo, arguments as any); | ||
}; | ||
}); |
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.
I got nothin'
…rappedApolloClient.tsx Co-authored-by: Jerel Miller <[email protected]>
As I don't want to change the shape of the DataTransportAbstraction immediately after we released it for the first time, this one would also need to go into 0.9.0.
This switches from two very separate "request started" and "request finished" events to a stream of events with a shared identifier between queries.
This has the benefit that in the future we can easily track which events are still "unhandled", e.g. for keeping the connection open for longer.
Also, for the first time we detect & communicate errors that happened in SSR to the browser, to restart queries there immediately.
As a positive side-effect of having a "query identifier", the query document is sent only once, not twice over. That already plays a bit into #210.
(Technically, it's sent one more time per hook, so we're down from three times to two).