add (TeeTo|ReadFrom)ReadableStreamLink links #387
Conversation
❌ Deploy Preview for apollo-client-nextjs-docmodel failed.
|
commit: |
size-limit report 📦
|
#339 Bundle Size — 1.25MiB (+0.06%).0cfc00a(current) vs 9d0a77a main#331(baseline) Warning Bundle contains 1 duplicate package – View duplicate packages Bundle metrics
|
| Current #339 |
Baseline #331 |
|
|---|---|---|
 Initial JS |
1017.58KiB(+0.08%) |
1016.77KiB |
 Initial CSS |
70B |
70B |
 Cache Invalidation |
22.06% |
0.08% |
| Chunks |
34 |
34 |
| Assets |
59 |
59 |
| Modules |
636 |
636 |
| Duplicate Modules |
106 |
106 |
| Duplicate Code |
4.66% |
4.66% |
| Packages |
26 |
26 |
| Duplicate Packages |
1 |
1 |
Bundle size by type 1 change 1 regression
| Current #339 |
Baseline #331 |
|
|---|---|---|
| JS |
1.24MiB (+0.06%) |
1.24MiB |
| Other |
9.09KiB |
9.09KiB |
| CSS |
70B |
70B |
Bundle analysis report Branch pr/framework-explorations Project dashboard
Generated by RelativeCI Documentation Report issue
|
|
||
| if (controller) { | ||
| return new Observable((observer) => { | ||
| const subscription = forward(operation).subscribe( |
There was a problem hiding this comment.
When we move to RxJS, this signature is deprecated. Can you use an observer object instead?
.subscribe({
next: () => {...},
error: () => {...},
complete: () => {...}
})| /** | ||
| * A link that allows the request to be cloned into a readable stream, e.g. for | ||
| * transport of multipart responses from RSC or a server loader to the browser. | ||
| */ | ||
| export const TeeToReadableStreamLink = new ApolloLink((operation, forward) => { |
There was a problem hiding this comment.
Looking at #389, it looks like these are used internally and not actually set by the user. If that is the case, should we mark these links as @internal? (same goes for the read link)
| /** | |
| * A link that allows the request to be cloned into a readable stream, e.g. for | |
| * transport of multipart responses from RSC or a server loader to the browser. | |
| */ | |
| export const TeeToReadableStreamLink = new ApolloLink((operation, forward) => { | |
| /** | |
| * A link that allows the request to be cloned into a readable stream, e.g. for | |
| * transport of multipart responses from RSC or a server loader to the browser. | |
| * | |
| * @internal | |
| */ | |
| export const TeeToReadableStreamLink = new ApolloLink((operation, forward) => { |
There was a problem hiding this comment.
I'd say that everything in this package could be considered "internal" or as a building block for "build-your-own-framework". These links will e.g. be used outside of the ApolloClient implementations of this package in the React-Router implementation.
| * @param context | ||
| * @returns | ||
| */ | ||
| export function readFromReadableStream<T extends Record<string, any>>( |
There was a problem hiding this comment.
Same question as the link. Should these be marked as @internal since they seem to be used internally in #389?
| } | ||
|
|
||
| const teeToReadableStreamKey = Symbol.for( | ||
| "apollo.tee.readableStreamController" |
There was a problem hiding this comment.
| "apollo.tee.readableStreamController" | |
| "apollo.readableStreamController" |
Is there anything else we'd consider putting on apollo.tee.*? If not, I'd simplify this to a single namespace (same goes for the read key)
There was a problem hiding this comment.
These are common enough that I see them colliding with something else in the future - it's so close to a web standard type that I would combine it with the method calling it: https://developer.mozilla.org/en-US/docs/Web/API/ReadableStreamDefaultController
🦋 Changeset detectedLatest commit: 0cfc00a The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This reverts commit 31b9d01.
No description provided.