Use ReadableStreamLinks for normal data transport, too #424
Conversation
|
commit: |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
#457 Bundle Size — 1.33MiB (+6.79%).49c78d2(current) vs 5293db1 main#409(baseline) Warning Bundle contains 1 duplicate package – View duplicate packages Warning Bundle introduced one new package: process – View changed packages Bundle metrics
|
| Current #457 |
Baseline #409 |
|
|---|---|---|
 Initial JS |
1.05MiB(+5.64%) |
1016.91KiB |
 Initial CSS |
70B |
70B |
 Cache Invalidation |
81.12% |
0.08% |
| Chunks |
37(+8.82%) |
34 |
| Assets |
63(+6.78%) |
59 |
| Modules |
676(+6.29%) |
636 |
 Duplicate Modules |
98(-7.55%) |
106 |
| Duplicate Code |
4.31%(-8.3%) |
4.7% |
| Packages |
25(-3.85%) |
26 |
| Duplicate Packages |
1 |
1 |
Bundle size by type 3 changes 3 regressions
| Current #457 |
Baseline #409 |
|
|---|---|---|
| JS |
1.32MiB (+100%) |
undefined |
| Other |
10.27KiB (+100%) |
undefined |
| CSS |
70B (+100%) |
undefined |
Bundle analysis report Branch pr/use-readable-stream-everywher... Project dashboard
Generated by RelativeCI Documentation Report issue
phryneas
force-pushed
the
pr/use-readable-stream-everywhere
branch
from
c4219d8 to
43ce452
Compare
phryneas
changed the title
| @@ -313,6 +315,7 @@ describe( | |||
| appendToBody`<div hidden id="S:1"><div id="user">User</div></div>`; | |||
| $RS("S:1", "P:1"); | |||
|
|
|||
| await new Promise((resolve) => setTimeout(resolve, 10)); | |||
There was a problem hiding this comment.
Arriving request data (simulateRequestData) now takes a tick as it goes through the link chain instead of directly being written to the store. Without this delay, this test would have written the query first and then the data from the link would have arrived.
| } | ||
|
|
||
| setLink(newLink: ApolloLink) { | ||
| super.setLink.call( | ||
| this, | ||
| ApolloLink.from([ | ||
| new ReadFromReadableStreamLink(), | ||
| new TeeToReadableStreamLink(), | ||
| newLink, | ||
| ]) | ||
| ); | ||
| } |
There was a problem hiding this comment.
All implementations (SSR, RSC, Browser) now use the same links - RSC could omit the ReadFromReadableStreamLink, but keeping all those separate details is very prone to subtle bugs; this is more reasonable.
| if ( | ||
| !queryManager["inFlightLinkObservables"].peekArray(cacheKeyArr) | ||
| ?.observable | ||
| ) { |
There was a problem hiding this comment.
No more manually added inFlightLinkObservables - we really use a link, so this doesn't need to fiddle with QueryManager internals.
| observable: streamObservable, | ||
| return super.watchQuery({ | ||
| ...options, | ||
| context: teeToReadableStream(() => this.pushEventStream(options), { |
There was a problem hiding this comment.
Moving this into a callback that only executes if the link is actually called means we will never inject a "started" event (or worse, a stream) when the query is deduplicated or for some other reason doesn't cause a network request.
| constructor(options: WrappedApolloClientOptions) { | ||
| super(options); | ||
| this.setLink(this.link); | ||
| } | ||
| class ApolloClientBrowserImpl extends ApolloClientClientBaseImpl {} | ||
|
|
||
| setLink(newLink: ApolloLink) { | ||
| super.setLink.call(this, new ReadFromReadableStreamLink().concat(newLink)); | ||
| } | ||
| } | ||
|
|
||
| class ApolloClientRSCImpl extends ApolloClientBase { | ||
| constructor(options: WrappedApolloClientOptions) { | ||
| super(options); | ||
| this.setLink(this.link); | ||
| } | ||
|
|
||
| setLink(newLink: ApolloLink) { | ||
| super.setLink.call(this, new TeeToReadableStreamLink().concat(newLink)); | ||
| } | ||
| } | ||
| class ApolloClientRSCImpl extends ApolloClientBase {} |
There was a problem hiding this comment.
Leaving these here even though they're empty - those class names might help in stack traces submitted by users to more quickly identify problems (and tbh., at some point I'll add back some env-specific logic here anyways)
| const script = `__APOLLO_EVENTS__.push(${jsesc(event, { | ||
| isScriptContext: true, | ||
| wrap: true, | ||
| json: true, | ||
| })})`; | ||
| ssr.injectScript(() => script); |
There was a problem hiding this comment.
This previously had a bug, where the injectScript callback would be called later and at that point in time the AC core had actually written additional properties to event, which meant that unneccessary data was transported over the wire.
We now stringify the object immediately.
No description provided.