build: Update api-extractor to 7.52.8#24599
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the version of @microsoft/api-extractor from 7.52.5 to 7.52.8 across the repository, addressing a regression where the --local flag did not correctly fail during outdated api report scenarios.
- Updated dependency version in multiple package.json files for API extractor.
- Adjusted patchedDependencies references to match the new version.
- Ensured consistency across both core and experimental packages.
Reviewed Changes
Copilot reviewed 94 out of 96 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/common/client-utils/package.json | Updated API extractor version to 7.52.8 |
| package.json | Updated API extractor version and patch reference to 7.52.8 |
| experimental/framework/tree-react-api/package.json | Updated API extractor version to 7.52.8 |
| experimental/framework/last-edited/package.json | Updated API extractor version to 7.52.8 |
| experimental/framework/data-objects/package.json | Updated API extractor version to 7.52.8 |
| experimental/dds/tree/package.json | Updated API extractor version to 7.52.8 |
| experimental/dds/sequence-deprecated/package.json | Updated API extractor version to 7.52.8 |
| experimental/dds/ot/sharejs/json1/package.json | Updated API extractor version to 7.52.8 |
| experimental/dds/ot/ot/package.json | Updated API extractor version to 7.52.8 |
| experimental/dds/attributable-map/package.json | Updated API extractor version to 7.52.8 |
| experimental/PropertyDDS/packages/property-dds/package.json | Updated API extractor version to 7.52.8 |
| experimental/PropertyDDS/packages/property-common/package.json | Updated API extractor version to 7.52.8 |
| examples/utils/migration-tools/package.json | Updated API extractor version to 7.52.8 |
| examples/utils/example-utils/package.json | Updated API extractor version to 7.52.8 |
| examples/data-objects/table-document/package.json | Updated API extractor version to 7.52.8 |
| common/lib/protocol-definitions/package.json | Updated API extractor version and patch reference to 7.52.8 |
| common/lib/common-utils/package.json | Updated API extractor version and patch reference to 7.52.8 |
| azure/packages/azure-service-utils/package.json | Updated API extractor version to 7.52.8 |
Files not reviewed (2)
- common/lib/common-utils/pnpm-lock.yaml: Language not supported
- common/lib/protocol-definitions/pnpm-lock.yaml: Language not supported
10efa0d to
e8847cb
Compare
e8847cb to
78bbaab
Compare
|
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output |
|
|
||
| ```ts | ||
|
|
||
| export { IMessageRelay } |
There was a problem hiding this comment.
This type has been @internal since its inception in 2023 and actually lives in @fluidframework/devtools-core, this package just re-exports it. Unclear why it was showing up here when it's internal, but it seems correct that it is not anymore.
FYI @Josmithr & @jason-ha, just informational since you both have a decent amount of knowledge/involvement with api-extractor.
There was a problem hiding this comment.
This is expected. I fixed this bug a week or two back, we just hadn't ingested the change yet. Glad it's working!
microsoft/rushstack#5211
| export { AsyncGenerator_2 as AsyncGenerator } | ||
|
|
||
| export { Generator_2 as Generator } |
There was a problem hiding this comment.
Similar scenario to comment above, these types are @internal so not sure what they were doing here.
There was a problem hiding this comment.
yeah. this package doesn't even need API reports really anyway.
There was a problem hiding this comment.
This was a bug fixed in microsoft/rushstack#5211
| readonly clientId: string; | ||
| } | ||
|
|
||
| export { IThrottlingWarning } |
There was a problem hiding this comment.
This one... the type definition itself (note, in @fluidframework/core-interfaces) is @legacy + @alpha. But when we re-export it from container-definitions, we provide new TSDoc that only specify @deprecated... is that overriding the original tags and making it disappear?
| export { getSerializedUploadedEditChunkContents } | ||
| export { getSerializedUploadedEditChunkContents as getUploadedEditChunkContents } |
There was a problem hiding this comment.
getSerializedUploadedEditChunkContents has been @internal ever since we created the tag, so seems they were incorrect in showing up here.
| export { IJSONRunSegment } | ||
|
|
||
| export { SharedSequence } | ||
|
|
||
| export { SubSequence } |
There was a problem hiding this comment.
@internal since we created the tag; why did we have several instances of those showing up here?
|
|
||
| ```ts | ||
|
|
||
| export { performance_2 as performance } |
There was a problem hiding this comment.
A bit confused as to why it called it performance_2 at any point... but the type is @internal so correct to disappear?
There was a problem hiding this comment.
api-extractor uses that convention when it needs to rename things for whatever reason (usually two conflicting imports in the api file). It should be transparent to users. No context on visibility of the type.
There was a problem hiding this comment.
Yeah, I'm just confused what the second thing was. Maybe the native performance object, I guess.
There was a problem hiding this comment.
Yup. API-Extractor renames it to avoid conflicting with the global, then re-exports it under the original alias.
|
|
||
| export { IUser } | ||
|
|
||
| export { ScopeType } |
There was a problem hiding this comment.
Change seems correct...? The type lives in @fluidframework/driver-definitions and it's @legacy + @alpha (separate API report, where it still shows up).
| export { BaseSegment } | ||
|
|
||
| export { InteriorSequencePlace } | ||
|
|
||
| export { ISegment } | ||
|
|
||
| export { LocalReferencePosition } | ||
|
|
||
| export { MapLike } | ||
|
|
||
| export { Marker } | ||
|
|
||
| export { MergeTreeDeltaType } | ||
|
|
||
| export { PropertySet } | ||
|
|
||
| export { ReferencePosition } | ||
|
|
||
| export { ReferenceType } | ||
|
|
||
| export { reservedMarkerIdKey } | ||
|
|
||
| export { SequencePlace } | ||
|
|
||
| export { Side } | ||
|
|
||
| export { TextSegment } | ||
|
|
||
| export { TrackingGroup } |
There was a problem hiding this comment.
These seem to be @legacy + @alpha which has its own api report, so correct they disappear here?
There was a problem hiding this comment.
yeah these should all be legacy+alpha
|
|
||
| ```ts | ||
|
|
||
| export { DocumentType_2 as DocumentType } |
There was a problem hiding this comment.
@internal, seems correct to go away.
| // @public | ||
| export type InsertableTypedNode<TSchema extends TreeNodeSchema, T = UnionToIntersection<TSchema>> = (T extends TreeNodeSchema<string, NodeKind, TreeNode | TreeLeafValue, never, true> ? NodeBuilderData<T> : never) | (T extends TreeNodeSchema ? Unhydrated<TreeNode extends NodeFromSchema<T> ? never : NodeFromSchema<T>> : never); | ||
|
|
||
| export { InteriorSequencePlace } |
There was a problem hiding this comment.
This and the two below are @legacy + @alpha (separate report) so correct to disappear?
CraigMacomber
left a comment
There was a problem hiding this comment.
Changes to Tree look good.
package.json and lock file changes also look good.
I'll leave opinions of some of the odd report deltas and the update to the patch (assuming it still works unchanged) for others to take a look at.
ChumpChief
left a comment
There was a problem hiding this comment.
Your explanations make sense to me I think. Might want an api-extractor expert to take a look but I don't see anything that worries me.
Abe27342
left a comment
There was a problem hiding this comment.
Types and changes I have context on look good to me. Maybe we want to do some more scrutiny in the area given this kind of surprising set of changes, but that can come later...
| /** | ||
| * A row in a table. | ||
| * @remarks Implemented by the schema class returned from {@link TableSchema.(createRow:2)}. | ||
| * @remarks Implemented by the schema class returned from {@link TableSchema.(row:2)}. |
There was a problem hiding this comment.
For context, this was a bug that I checked in. It was not caught due to the API-Extractor regression this PR is aimed to resolve.
Josmithr
left a comment
There was a problem hiding this comment.
So happy to see all of the API report lies go away! Thanks for updating!!
Description
Updates to version 7.52.8 of
@microsoft/api-extractorto fix a regression introduced in 7.52.5 that caused running it without the--localflag (like our pipelines) to not fail when it should have (e.g. if the api reports were out of date, or TSDoc links pointed to non-existent APIs).Also fixes some issues with api-reports that slipped in while the regression was in place (see 78bbaab).
Reviewer Guidance
The review process is outlined on this wiki page.
AB#37811