-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Conversation
|
// @public (undocumented) | ||
export function loadDevMessages(): void; | ||
|
||
// Warning: (ae-forgotten-export) The symbol "ErrorCodes" needs to be exported by the entry point index.d.ts |
There was a problem hiding this comment.
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.
size-limit report 📦
|
There was a problem hiding this 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!
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
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.