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

Add API Extractor API reports as a CI check #11215

Merged
merged 6 commits into from Sep 14, 2023
Merged

Add API Extractor API reports as a CI check #11215

merged 6 commits into from Sep 14, 2023

Conversation

phryneas
Copy link
Member

@phryneas phryneas commented Sep 14, 2023

This PR adds API Extractor API reports to our CI checks.

You can read up on API reports here: https://api-extractor.com/pages/overview/demo_api_report/

TLDR:

Api Extractor creates a bunch of Markdown files describing the "outer type shape" of Apollo Client - this includes all internal types that are referenced by public APIs, but excludes all "really internal" types we have.

On opening a PR, it recreates these files in CI and compares them to the committed files. If they differ, CI fails.

So for us, if we are confident that we want to ship a change to the "outer shape" of our package, we run

npm run build
npm run extract-api

locally and commit the changed Markdown files.

That shows API extractor that we are aware of these changes, and they are intended.

If we do not do this, chances are we accidentally introduced a change (that might be breaking) and need to review these changes (and then either roll them back or decide that they are okay and add an updated report to the PR).

I will be rebasing #11200 onto this branch, as I want to be double-sure that we don't accidentally add breaking changes in that PR and that we are aware of any potential changes we introduce there.

@changeset-bot
Copy link

changeset-bot bot commented Sep 14, 2023

⚠️ No Changeset found

Latest commit: ebad71b

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

// @public (undocumented)
export function loadDevMessages(): void;

// Warning: (ae-forgotten-export) The symbol "ErrorCodes" needs to be exported by the entry point index.d.ts
Copy link
Member Author

Choose a reason for hiding this comment

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

You can read up on this warning here:

https://api-extractor.com/pages/messages/ae-forgotten-export/

TLDR: The type is not public, but used by a public export.
That means multiple things:

  • if it points to a third-party package, consumers of ours that use declaration: true, they might get a TS build error (along the lines of "export cannot be named")
  • it might be fiddly to type

On the other hand, it might be perfectly fine - we shouldn't add every helper type to our public api, as it would add tons of stuff to keep supporting for backwards compatibility.

=> The warnings are here, we should consider them on a per-case basis (mostly when we add new stuff), but not take them too seriously.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 14, 2023

size-limit report 📦

Path Size
dist/apollo-client.min.cjs 37.28 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" 43.75 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production) 42.32 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" 32.52 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production) 31.27 KB (0%)
import { ApolloProvider } from "dist/react/index.js" 1.21 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production) 1.2 KB (0%)
import { useQuery } from "dist/react/index.js" 4.27 KB (0%)
import { useQuery } from "dist/react/index.js" (production) 4.08 KB (0%)
import { useLazyQuery } from "dist/react/index.js" 4.58 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production) 4.4 KB (0%)
import { useMutation } from "dist/react/index.js" 2.52 KB (0%)
import { useMutation } from "dist/react/index.js" (production) 2.51 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.76 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production) 4.19 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" 4.28 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production) 3.7 KB (0%)
import { useReadQuery } from "dist/react/index.js" 2.96 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production) 2.91 KB (0%)
import { useFragment } from "dist/react/index.js" 2.08 KB (0%)
import { useFragment } from "dist/react/index.js" (production) 2.03 KB (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.

This is pretty sweet! Great find and will definitely be useful!

@phryneas phryneas merged commit c7a1b71 into main Sep 14, 2023
22 checks passed
@phryneas phryneas deleted the pr/api-extractor branch September 14, 2023 16:27
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants