Skip to content
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 (8) documentation updates and "branded" error messages #240

Merged
merged 17 commits into from
Mar 21, 2024

Conversation

phryneas
Copy link
Member

This PR contains documentation updates and "branded" error messages to ensure that error messages match the package that has been imported.

Copy link

relativeci bot commented Mar 13, 2024

Job #34: Bundle Size — 1.01MiB (+0.04%).

2638231(current) vs b63b501 main#30(baseline)

Warning

Bundle contains 1 duplicate package – View duplicate packages

Bundle metrics  Change 4 changes Regression 1 regression Improvement 1 improvement
                 Current
Job #34
     Baseline
Job #30
Regression  Initial JS 890.09KiB(+0.05%) 889.69KiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 35.03% 21.01%
No change  Chunks 24 24
No change  Assets 45 45
Change  Modules 512(+0.2%) 511
No change  Duplicate Modules 30 30
Improvement  Duplicate Code 1.29%(-0.77%) 1.3%
No change  Packages 29 29
No change  Duplicate Packages 1 1
Bundle size by type  Change 1 change Regression 1 regression
                 Current
Job #34
     Baseline
Job #30
Regression  JS 1023.35KiB (+0.04%) 1022.95KiB
Not changed  Other 5.99KiB 5.99KiB

View job #34 reportView pr/0.8-docs branch activityView project dashboard

@phryneas phryneas added this to the 0.9.0 milestone Mar 14, 2024
@phryneas phryneas changed the title 0.10.0 (8) documentation updates and "branded" error messages 0.9.0 (8) documentation updates and "branded" error messages Mar 14, 2024
Comment on lines +33 to +38
- name: Commit changes back
uses: stefanzweifel/git-auto-commit-action@v5
with:
commit_message: "Update Docs"
push_options: ""
skip_dirty_check: false
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This builds the docs folder and re-commits it to main if any changes are necessary.

I tried building to HTML with the intention of deploying to GH pages, but that would require us to also come up with a design for those pages, while "markdown on GitHub" already has some kind of familiar frame around it.


## client-react-streaming package

This package provides building blocks to create framework-level integration of Apollo Client with React's streaming SSR. See the \[@<!-- -->apollo/experimental-nextjs-app-support\](https://github.com/apollographql/apollo-client-nextjs/tree/main/packages/experimental-nextjs-app-support) package as an example.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason, some of the Markdown links from comments are escaped like this.

It's not pretty, but tbh., I'm fine with it.

@@ -24,7 +24,17 @@ afterEach(resetNextSSRApolloSingletons);
* data from the server to the browser.
*/
test("uses the browser build", () => {
expect(NextSSRApolloClient.name).toBe("ApolloClientBrowserImpl");
let foundPrototype = false;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the class hierarchy of ApolloClient has changed a bit with this PR, this test had to get a bit more complicated.

@@ -0,0 +1,101 @@
import assert from "node:assert";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have a test setup over in @apollo/experimental-nextjs-app-support, so we're testing this only over in this package.

Long-term it might be a good idea to mirror the test over there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I just tried it out, just in case:
One works:
image

The other one doesn't yet - I'll get back on that.
image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking better now!
image
image

@@ -0,0 +1,6 @@
/**
* @packageDocumentation
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing here yet, but we have a very long README.

@phryneas
Copy link
Member Author

/release:pr

Copy link

A new release has been made for this PR. You can install the package you need using one of

  • npm i @apollo/experimental-nextjs-app-support@0.0.0-commit-4fc1615
  • npm i @apollo/client-react-streaming@0.0.0-commit-4fc1615

@phryneas
Copy link
Member Author

/release:pr

Copy link

A new release has been made for this PR. You can install the package you need using one of

  • npm i @apollo/experimental-nextjs-app-support@0.0.0-commit-e67a320
  • npm i @apollo/client-react-streaming@0.0.0-commit-e67a320

@phryneas phryneas marked this pull request as ready for review March 15, 2024 14:39
@phryneas phryneas requested a review from a team as a code owner March 15, 2024 14:39
Copy link

github-actions bot commented Mar 15, 2024

size-limit report 📦

Path Size
{ ApolloNextAppProvider, NextSSRApolloClient, NextSSRInMemoryCache } from '@apollo/experimental-nextjs-app-support/ssr' (Browser ESM) 7.78 KB (0%)
{ WrapApolloProvider, ApolloClient, InMemoryCache } from '@apollo/client-react-streaming' (Browser ESM) 1.4 KB (0%)
{ buildManualDataTransport } from '@apollo/client-react-streaming/manual-transport' (Browser ESM) 6.24 KB (0%)
@apollo/client-react-streaming (Browser ESM) 2.11 KB (0%)
@apollo/client-react-streaming (SSR ESM) 1.72 KB (0%)
@apollo/client-react-streaming (RSC ESM) 1020 B (0%)
@apollo/client-react-streaming/manual-transport (Browser ESM) 6.42 KB (0%)
@apollo/client-react-streaming/manual-transport (SSR ESM) 6.32 KB (0%)
@apollo/experimental-nextjs-app-support/ssr (Browser ESM) 8.39 KB (0%)
@apollo/experimental-nextjs-app-support/ssr (SSR ESM) 8.3 KB (0%)
@apollo/experimental-nextjs-app-support/ssr (RSC ESM) 839 B (0%)
@apollo/experimental-nextjs-app-support/rsc (RSC ESM) 261 B (0%)

Copy link
Member

@jerelmiller jerelmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this took me so long to finish up! Looking awesome!

*
* While this Data Transport enables streaming SSR, it has some conceptual drawbacks:
*
* - It does not have a way of keeping your connection open if your application already finished, but there are still ongoing queries that might need to be awaited transported over.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* - It does not have a way of keeping your connection open if your application already finished, but there are still ongoing queries that might need to be awaited transported over.
* - It does not have a way of keeping your connection open if your application already finished, but there are still ongoing queries that might need to be transported over.

*
* - It does not have a way of keeping your connection open if your application already finished, but there are still ongoing queries that might need to be awaited transported over.
* - This can happen if a component renders `useBackgroundQuery`, but does not read the `queryRef` with `useReadQuery`
* - These "cut off" queries will be restarted in the Browser once the browser's `load` event happens
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* - These "cut off" queries will be restarted in the Browser once the browser's `load` event happens
* - These "cut off" queries will be restarted in the browser once the browser's `load` event happens

Comment on lines 140 to 141
* - In this, case, older data from the server might overwrite newer data in the browser.
* - This is minimized by simulating ongoing queries in the browser once the information of a started query is transported over.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* - In this, case, older data from the server might overwrite newer data in the browser.
* - This is minimized by simulating ongoing queries in the browser once the information of a started query is transported over.
* - In this, case, older data from the server might overwrite newer data in the browser. This is minimized by simulating ongoing queries in the browser once the information of a started query is transported over.

The 2nd sentence feels more like a continuation of the third, not so much its own point, so I think combining these makes sense.

@phryneas phryneas merged commit da238f8 into main Mar 21, 2024
4 of 6 checks passed
@phryneas phryneas deleted the pr/0.8-docs branch March 21, 2024 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants