disable `removeComments`, add docmodel, inheritDoc processing #11290

phryneas · 2023-10-16T10:18:01Z

I just noticed this on accident - apparently we have been stripping all our comments, which also removed DocBlocks and left our users without documentation in their IDE, even if we had written it for them.

This also means that none of our `@deprecated` annotations ever had a chance to reach our users

If you look at the Build Output Comparison, a few things come to mind:

.js files now also preserve comments - including non-docblock comments. Without an additional build step, there's no way around that.
.cjs files have a few different line breaks. I couldn't find a tool that would remove comments in the exact same way.
.d.ts files now have docblocks. That's what we want.

Please review, but don't merge yet.

This also needs a review over in apollographql/docs#588

Checklist:

If this PR contains changes to the library itself (not necessary for e.g. docs updates), please include a changeset (see CONTRIBUTING.md)
If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
Make sure all of the significant new logic is covered by tests

changeset-bot · 2023-10-16T10:18:04Z

⚠️ No Changeset found

Latest commit: 77220fe

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions · 2023-10-16T10:19:47Z

size-limit report 📦

Path	Size
dist/apollo-client.min.cjs	37.08 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs"	43.51 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production)	42.01 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js"	32.61 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production)	31.27 KB (0%)
import { ApolloProvider } from "dist/react/index.js"	1.22 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production)	1.21 KB (0%)
import { useQuery } from "dist/react/index.js"	4.28 KB (0%)
import { useQuery } from "dist/react/index.js" (production)	4.1 KB (0%)
import { useLazyQuery } from "dist/react/index.js"	4.59 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production)	4.41 KB (0%)
import { useMutation } from "dist/react/index.js"	2.55 KB (0%)
import { useMutation } from "dist/react/index.js" (production)	2.54 KB (0%)
import { useSubscription } from "dist/react/index.js"	2.24 KB (0%)
import { useSubscription } from "dist/react/index.js" (production)	2.2 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js"	4.62 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production)	4.05 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js"	4.14 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production)	3.56 KB (0%)
import { useReadQuery } from "dist/react/index.js"	3 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production)	2.94 KB (0%)
import { useFragment } from "dist/react/index.js"	2.09 KB (0%)
import { useFragment } from "dist/react/index.js" (production)	2.04 KB (0%)

netlify · 2023-10-16T14:24:53Z

✅ Deploy Preview for apollo-client-docs ready!

Name	Link
🔨 Latest commit	`893d1df`
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-client-docs/deploys/655c8959d11eb0000827bbc6
😎 Deploy Preview	https://deploy-preview-11290--apollo-client-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

phryneas · 2023-10-31T17:51:09Z

Okay, trying to get some short explanations in here.

A Docblock in TSDoc has a few "sections":

  /**
   * Returns the average of two numbers.
   *
   * @remarks
   * This method is part of the {@link core-library#Statistics | Statistics subsystem}.
   *
   * @param x - The first input number
   * @param y - The second input number
   * @returns The arithmetic mean of `x` and `y`
   * @example
   * Prints "true" for `{@link}` but "false" for `@internal`:
   * ```ts
   * console.log(isInlineTag('{@link}'));
   * console.log(isInlineTag('@internal'));
   * ```
   * @see {@link http://example.com/@internal | the @internal tag}
   * @beta
   */
function between(a: number, b: number): number{
}

We have a "summary", which is everything up to the first different block, then we have a "remarks" block, some @param, @returns, @see and @beta annotations and an @example block

Block	Contents
summary	Returns the average of two numbers.
remarks	This method is part of the {@link core-library#Statistics
example	Prints "true" for `{@link}` but "false" for `@internal`:, followed by the example code

We can reference those from the documentation:

<DocBlock canonicalReference="@apollo/client!ApolloClient:constructor(1)" />

would render by default the summary, and if there are any @since or @deprecated tags, those before that.
That can be adjusted:

<DocBlock summary={false} remark  canonicalReference="@apollo/client!ApolloClient:constructor(1)" />

Adding the remark prop per default renders the remark after the summary, as a collapsible, but the additional prop remarkCollapsible={false} would just render it inline.

Options here are:

export function DocBlock({
  canonicalReference,
  summary = true,
  remark = false,
  example = false,
  remarkCollapsible = true,
  since = true,
  deprecated = true
})

If we just need a little piece, we can also use DocPiece which can always only render one piece of docs. Every piece can be rendered collapsible, so e.g.

<DocPiece summary canonicalReference="@apollo/client!ApolloClient:constructor(1)" />
<DocPiece remark canonicalReference="@apollo/client!ApolloClient:constructor(1)" />
<DocPiece example collapsible canonicalReference="@apollo/client!ApolloClient:constructor(1)" />

Apart from that, we currently have these components (but can add more!)

<FunctionDetails canonicalReference="@apollo/client!ApolloClient#watchQuery:member(1)" />
<InterfaceDetails canonicalReference="@apollo/client!ApolloClientOptions:interface" />
// can be configured a bit:
<InterfaceDetails canonicalReference="@apollo/client!ApolloClientOptions:interface" headingLevel={4} /* size of the heading*/ link={false} /* if the heading should be a link anchor */ />

phryneas · 2023-10-31T17:55:28Z

src/core/watchQueryOptions.ts

@@ -118,7 +118,7 @@ export interface QueryOptions<TVariables = OperationVariables, TData = any> {
 export interface WatchQueryOptions<
  TVariables extends OperationVariables = OperationVariables,
  TData = any,
-> extends Omit<QueryOptions<TVariables, TData>, "fetchPolicy"> {
+> {


This interface had "complicated inheritance", so inherited members cannot be automatically determined - we have to inline them.
(We could alternatively create a supertype without fetchPolicy and inherit from that)

phryneas · 2023-10-31T17:56:25Z

src/core/watchQueryOptions.ts

@@ -149,6 +149,33 @@ export interface WatchQueryOptions<
   * behavior, for backwards compatibility with Apollo Client 3.x.
   */
  refetchWritePolicy?: RefetchWritePolicy;
+
+  /** {@inheritDoc @apollo/client!QueryOptions#query:member} */


This will be inlined from the comment on QueryOptions.query during build time and on docmodel generation, to prevent duplication of identical comments - @jerelmiller this will come very handy with your plan of flattening interface hierarchies.

phryneas · 2023-10-31T17:57:33Z

src/core/ApolloClient.ts

+   * @example
+   * ```js
+   * import { ApolloClient, InMemoryCache } from '@apollo/client';


This example moved into the code and is now just referenced from the docs.

netlify · 2023-10-31T19:14:31Z

Comment has been resolved by Lenz Weber-Tronic

Lenz Weber-Tronic left a comment:

I know about the stray @example - I'll remove it once I'm back from my day off 😃

Browser metadata

Path:      /api/core/ApolloClient
Browser:   Firefox 118.0 on Mac OS 10.15
Viewport:  3428 x 1291 @1x
Language:  de-DE
Cookies:   Enabled

Open in BrowserStack

Open Deploy Preview · Mark as Unresolved

phryneas · 2023-11-03T16:48:33Z

.gitignore

@@ -73,3 +73,4 @@ esbuild-why-*.html

 .yalc
 yalc.lock
+docs/shared/client.api.json


This file is autogenerated on a bunch of occasions and doesn't need to be checked in.

phryneas · 2023-11-03T16:48:58Z

api-extractor.json

@@ -37,14 +38,17 @@
  },

  "docModel": {
-    "enabled": false
+    "enabled": false,


We are now also generating a docModel for consumption in the docs build.

phryneas · 2023-11-03T16:49:40Z

config/apiExtractor.ts

+const parsed = parseArgs({
+  options: {
+    generate: {
+      type: "string",
+      multiple: true,
+      default: ["apiReport"],
+    },
+    "main-only": {
+      type: "boolean",
+      default: false,
+    },
+  },
+});


Adding some new command line arguments that allow us to use this tool in a few different ways.

phryneas · 2023-11-03T16:51:56Z

config/inlineInheritDoc.ts

+console.log(
+  "Processing {@inheritDoc <canonicalReference>} comments in .d.ts files."
+);


This CLI tool will inline docblocks specified with @inheritDoc on build. We need this here for situations where inheritance is not possible for docModel generation (interface Foo extends Omit<Bar, 'baz'> {} is too complicated a type for it to parse) and we want to flatten types - or going forward, for generally flattening types without repeating docs everywhere.

This is super useful information. Would it be useful to add this as a code comment in this file? I think future us could benefit from this information in case we stumble across this script again and wonder why its needed.

Added a comment at the top of the file :)

phryneas · 2023-11-03T16:52:30Z

config/inlineInheritDoc.ts

+    Extractor.invoke(extractorConfig);
+
+    const model = new ApiModel();
+    model.loadPackage(tempModelFile);


There's unfortunately no better way than writing and immediately reading this file 🤦

phryneas · 2023-11-03T16:54:20Z

config/rollup.config.js


 const entryPoints = require("./entryPoints");
 const distDir = "./dist";

+const removeComments = cleanup({});


In .js rollup builds we still want to remove comments - they only increase bundle size here and we only need them in .d.ts files.

They will still be in the ESM files directly built by tsc, but these will always be used by some kind of bundler on user side, so it's okay.

phryneas · 2023-11-03T16:55:12Z

docs/source/api/core/ApolloClient.mdx

+api_doc:
+  - "@apollo/client!ApolloClient:class"
+  - "@apollo/client!DefaultOptions:interface"


We need to reference the "parent" entities of things we want to document on this page, so they are available for the DocBlock etc. components later on.

phryneas · 2023-11-03T16:55:58Z

netlify.toml

  cd ../
  rm -rf monodocs
-  git clone https://github.com/apollographql/docs --branch main --single-branch monodocs
+  git clone https://github.com/apollographql/docs --branch pr/apidoc --single-branch monodocs


This needs to change once apollographql/docs#588 has been merged - we'll have to coordinate both of those PRs.

phryneas · 2023-11-03T16:58:03Z

src/testing/core/observableToPromise.ts

-/**
- *
- * @param observable the observable query to subscribe to
- * @param shouldResolve should we resolve after seeing all our callbacks [default: true]
- *   (use this if you are racing the promise against another)
- * @param wait how long to wait after seeing desired callbacks before resolving
- *   [default: -1 => don't wait]
- * @param errorCallbacks an expected set of errors
- */
-export type Options = {


These are not parameters, but interface members => fixing up the docblocks.

phryneas · 2023-11-03T16:58:34Z

tsconfig.json

@@ -7,7 +7,6 @@
    "skipLibCheck": true,
    "moduleResolution": "node",
    "importHelpers": true,
-    "removeComments": true,


This is the one line that triggered this rabbit hole 🐇

phryneas · 2023-11-14T14:46:03Z

apollographql/docs#588 has been reviewed and is approved.

jerelmiller · 2023-11-14T14:48:43Z

config/apiExtractor.ts

 } from "@microsoft/api-extractor";
+import { parseArgs } from "node:util";


TIL Node has a parseArgs utility. Neat!

jerelmiller

Nice work! Will be great to have some better DX when using the client. Hard to believe we've gone this long and not realized this has been a problem.

config/inlineInheritDoc.ts

jerelmiller · 2023-11-20T18:39:23Z

src/core/ApolloClient.ts

+  /**
+   * If `true`, the [Apollo Client Devtools](https://www.apollographql.com/docs/react/development-testing/developer-tooling/#apollo-client-devtools) browser extension can connect to Apollo Client in your production environment. The extension can _always_ connect in a non-production environment.
+   *
+   * The default value is `false`.


Perhaps I'm being a bit overly detailed here, but the default value looks to be inferred on whether we are in a browser environment in development mode. Should this be a bit closer to actual, or leave it as false here? Also, should this be a @defaultValue?

Also, should this be a @DefaultValue

I believe not, since this is an interface and not a function parameter.

Perhaps I'm being a bit overly detailed here, but the default value looks to be inferred on whether we are in a browser environment in development mode.

Good point, I believe the whole long description here was wrong, with statements that it would always be allowed in Dev, even if this were false. Adjusted.

src/core/ApolloClient.ts

Co-authored-by: Jerel Miller <[email protected]>

disable removeComments

04a040a

phryneas added 4 commits October 16, 2023 12:31

rollup adjustments

eddb8cc

use rollup-plugin-cleanup

f4cd508

adjust docblocks, new api report

65e8386

update docblocks to match with docs

10b5b48

try new InterfaceTable docs component

4fe9ba7

phryneas mentioned this pull request Oct 25, 2023

Change from typedoc to tsdoc/api-extractor-model apollographql/docs#588

Merged

phryneas self-assigned this Oct 25, 2023

phryneas added 6 commits October 25, 2023 16:38

preview build changes

9199450

bump CI

99815c0

some example usages

da2fea3

do not pack .d.ts.map in the package

97c6121

add support for inlining @inheritdoc comments

127c7ad

use new components

b16f8b5

phryneas commented Oct 31, 2023

View reviewed changes

phryneas added 8 commits November 3, 2023 10:38

reduce required frontmatter

5d364ed

bump CI

f9c9ffe

bump CI

e554377

use customPropertyOrder

35db4d6

move incorrect @params into correct positions

3e5e229

api-reports

68eccdc

formatting

ad77545

actually trigger docs deploy

1815640

phryneas commented Nov 3, 2023

View reviewed changes

phryneas marked this pull request as ready for review November 3, 2023 16:58

phryneas requested a review from a team as a code owner November 3, 2023 16:58

phryneas requested review from jerelmiller and alessbell November 3, 2023 16:59

jerelmiller reviewed Nov 14, 2023

View reviewed changes

add explainer comment

2a26d9d

jerelmiller approved these changes Nov 20, 2023

View reviewed changes

phryneas added the auto-cleanup 🤖 label Nov 21, 2023

phryneas and others added 5 commits November 21, 2023 11:39

Merge remote-tracking branch 'origin/main' into pr/comments

a312a44

Clean up Prettier, Size-limit, and Api-Extractor

893d1df

Apply suggestions from code review

7a16383

Co-authored-by: Jerel Miller <[email protected]>

update comment

56fdb44

preparations for deploy

77220fe

phryneas changed the title ~~disable removeComments~~ disable removeComments, add docmodel, inheritDoc processing Nov 22, 2023

phryneas merged commit dc67e24 into main Nov 22, 2023
16 of 21 checks passed

phryneas deleted the pr/comments branch November 22, 2023 10:33

github-actions bot locked as resolved and limited conversation to collaborators Dec 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

disable `removeComments`, add docmodel, inheritDoc processing #11290

disable `removeComments`, add docmodel, inheritDoc processing #11290

phryneas commented Oct 16, 2023 •

edited

Loading

changeset-bot bot commented Oct 16, 2023 •

edited

Loading

github-actions bot commented Oct 16, 2023 •

edited

Loading

netlify bot commented Oct 16, 2023 •

edited

Loading

phryneas commented Oct 31, 2023 •

edited

Loading

phryneas Oct 31, 2023 •

edited

Loading

phryneas Oct 31, 2023 •

edited

Loading

phryneas Oct 31, 2023

netlify bot commented Oct 31, 2023 •

edited

Loading

phryneas Nov 3, 2023

phryneas Nov 3, 2023

phryneas Nov 3, 2023

phryneas Nov 3, 2023

jerelmiller Nov 14, 2023 •

edited

Loading

phryneas Nov 16, 2023

phryneas Nov 3, 2023

phryneas Nov 3, 2023

phryneas Nov 3, 2023

phryneas Nov 3, 2023 •

edited

Loading

phryneas Nov 3, 2023

phryneas Nov 3, 2023

phryneas commented Nov 14, 2023

jerelmiller Nov 14, 2023

jerelmiller left a comment

jerelmiller Nov 20, 2023

phryneas Nov 21, 2023

		} from "@microsoft/api-extractor";
		import { parseArgs } from "node:util";

disable removeComments, add docmodel, inheritDoc processing #11290

disable removeComments, add docmodel, inheritDoc processing #11290

Conversation

phryneas commented Oct 16, 2023 • edited Loading

This also means that none of our @deprecated annotations ever had a chance to reach our users

Please review, but don't merge yet.

Checklist:

changeset-bot bot commented Oct 16, 2023 • edited Loading

⚠️ No Changeset found

github-actions bot commented Oct 16, 2023 • edited Loading

size-limit report 📦

netlify bot commented Oct 16, 2023 • edited Loading

✅ Deploy Preview for apollo-client-docs ready!

phryneas commented Oct 31, 2023 • edited Loading

phryneas Oct 31, 2023 • edited Loading

Choose a reason for hiding this comment

phryneas Oct 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Oct 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerelmiller Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phryneas Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phryneas commented Nov 14, 2023

Choose a reason for hiding this comment

jerelmiller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

disable `removeComments`, add docmodel, inheritDoc processing #11290

disable `removeComments`, add docmodel, inheritDoc processing #11290

phryneas commented Oct 16, 2023 •

edited

Loading

This also means that none of our `@deprecated` annotations ever had a chance to reach our users

changeset-bot bot commented Oct 16, 2023 •

edited

Loading

github-actions bot commented Oct 16, 2023 •

edited

Loading

netlify bot commented Oct 16, 2023 •

edited

Loading

phryneas commented Oct 31, 2023 •

edited

Loading

phryneas Oct 31, 2023 •

edited

Loading

phryneas Oct 31, 2023 •

edited

Loading

netlify bot commented Oct 31, 2023 •

edited

Loading

jerelmiller Nov 14, 2023 •

edited

Loading

phryneas Nov 3, 2023 •

edited

Loading