pnpm dedupe #25559

CraigMacomber · 2025-09-26T00:42:00Z

Description

pnpm dedupe in all packages.

This can almost be done with:

pnpm exec flub exec "pnpm dedupe --ignore-scripts" --releaseGroupRoot=all --packages

But that had some issues: server failed due to failing to apply the old version of the api-extractor patch, so I manually updated that to match what client uses.

Even after that pnpm exec flub exec "pnpm dedupe --ignore-scripts" --releaseGroupRoot=all --packages was failing since it seemed to try and do client twice at once.

Also updates @octokit/core and @octokit/rest to the latest versions to fix a compile error that happened after dedupe.

Updated api-extractor used in server/routerlicious to match client since deupe updated it which broke the patch we use.

Updated node types package overrider in server/routerlicious to fix build error from conflicting node types globals.

Reviewer Guidance

The review process is outlined on this wiki page.

Copilot

Pull Request Overview

This PR updates the Microsoft API Extractor dependency from version 7.45.1 to 7.52.11 across all server packages as part of a pnpm dedupe operation. The update includes creating a new patch file for the newer version and removing the old patch file.

Key changes:

Updates @microsoft/api-extractor from ^7.45.1 to ^7.52.11 in all affected packages
Creates a new patch file for version 7.52.11 with updated content
Removes the old patch file for version 7.45.1

Reviewed Changes

Copilot reviewed 7 out of 19 changed files in this pull request and generated 2 comments.

File	Description
server/routerlicious/patches/@microsoft__api-extractor@7.52.11.patch	New patch file for API extractor 7.52.11 with bug mitigation and import emission changes
server/routerlicious/patches/@microsoft__api-extractor@7.45.1.patch	Removed old patch file for API extractor 7.45.1
server/routerlicious/packages/*/package.json	Updated API extractor dependency version from 7.45.1 to 7.52.11
server/routerlicious/package.json	Updated root package dependency and patch configuration

Files not reviewed (11)

build-tools/pnpm-lock.yaml: Language not supported
common/build/eslint-config-fluid/pnpm-lock.yaml: Language not supported
common/lib/common-utils/pnpm-lock.yaml: Language not supported
common/lib/protocol-definitions/pnpm-lock.yaml: Language not supported
pnpm-lock.yaml: Language not supported
server/gitrest/pnpm-lock.yaml: Language not supported
server/historian/pnpm-lock.yaml: Language not supported
server/routerlicious/pnpm-lock.yaml: Language not supported
tools/benchmark/pnpm-lock.yaml: Language not supported
tools/getkeys/pnpm-lock.yaml: Language not supported
tools/test-tools/pnpm-lock.yaml: Language not supported

Copilot · 2025-09-26T00:44:26Z

server/routerlicious/patches/@microsoft__api-extractor@7.52.11.patch

+                 continue;
+             }
+-            if (collectorEntity && collectorEntity.consumable) {
+            // BUG MITIGATION: Always check release tag compatibility.


[nitpick] The comment 'BUG MITIGATION: Always check release tag compatibility' should be more descriptive. Consider explaining what specific bug this mitigates and why the original conditional logic was problematic.

Suggested change

+ // BUG MITIGATION: Always check release tag compatibility.

+ // BUG MITIGATION: Previously, release tag compatibility was only checked when collectorEntity.consumable was true.

+ // This led to missed analyzer issues when entities were not marked as consumable, causing incorrect API surface validation.

+ // To ensure all relevant symbols are checked for release tag compatibility, this block always performs the comparison,

+ // regardless of the consumable status. This mitigates bugs where incompatible release tags were not reported.

Copilot · 2025-09-26T00:44:27Z

server/routerlicious/patches/@microsoft__api-extractor@7.52.11.patch

+-        }
+-        writer.ensureSkippedLine();
+
+        // PATCH: Don't emit imports


[nitpick] The comment 'PATCH: Don't emit imports' should explain why imports are not being emitted. This could help future maintainers understand the reasoning behind this patch modification.

Suggested change

+ // PATCH: Don't emit imports

+ // PATCH: Don't emit imports.

+ // This patch disables emitting import statements in the generated API report.

+ // Rationale: Emitting imports can cause issues with downstream consumers of the API report,

+ // such as breaking type resolution or introducing unwanted dependencies. By omitting imports,

+ // we ensure the API report remains self-contained and compatible with our tooling and usage patterns.

CraigMacomber · 2025-09-26T03:17:00Z

Our build tools depend on otokit (for acesssing github APIs) for some reason, and after the dedup it was giving a bunch of errors like:

@fluid-tools/build-cli: ../../node_modules/.pnpm/@octokit+plugin-paginate-rest@11.3.3_@octokit+core@6.1.2/node_modules/@octokit/plugin-paginate-rest/dist-types/generated/paginating-endpoints.d.ts:56:31 - error TS2339: Property 'GET /enterprises/{enterprise}/copilot/usage' does not exist on type 'Endpoints'.
@fluid-tools/build-cli:
@fluid-tools/build-cli: 56 parameters: Endpoints["GET /enterprises/{enterprise}/copilot/usage"]["parameters"];

Updating @octokit/core and @octokit/rest to the latest versions seems to have fixed it.

CraigMacomber · 2025-09-26T17:20:01Z

server/routerlicious/package.json

 		"@fluidframework/build-common": "^2.0.3",
 		"@fluidframework/build-tools": "^0.57.0",
-		"@microsoft/api-extractor": "^7.45.1",
+		"@microsoft/api-extractor": "^7.52.11",


The dedup caused the exact version of @microsoft/api-extractor to change, and since we have a patch for it, that broke.

To fix it I just updated the version to match client, which also removed the imports from the API reports.

CraigMacomber · 2025-09-26T17:21:31Z

server/routerlicious/package.json

 		],
 		"overrides": {
-			"@types/node@<18": "^18.19.39",
+			"@types/node": "~20.0.0",


Without this change, the build was filing with a compile error from conflicting global.

I made this specific change to align with the readme's required node version.

Annoyingly this doesn't seem to fully work as the lock file still has another version in it, but it fixed it enough for the build to work.

CraigMacomber · 2025-09-26T17:26:08Z

server/routerlicious/pnpm-lock.yaml

-  '@types/node@18.19.39':
-    resolution: {integrity: sha512-nPwTRDKUctxw3di5b4TfT3I0sWDiWoPQCZjXhvdkINntwr8lcoVCKsTgnXeRubKIlfnV+eN/HYk6Jb40tbcEAQ==}
+  '@types/node@20.0.0':
+    resolution: {integrity: sha512-cD2uPTDnQQCVpmRefonO98/PPijuOnnEy5oytWJFPY1N9aJCz2wJ5kSGWO+zJoed2cY2JxQh6yBuUq4vIn61hw==}

  '@types/node@22.10.7':


The override in the package.json should have removed this, but it didn't. The override was enough to fix the build error though, so I guess this is ok.

I was wrong: the ci docs build still fails, I assume due to this:

packages/services-client ci:build:docs: Error: /home/craig/Work/FluidFramework/server/routerlicious/node_modules/.pnpm/@types+node@20.0.0/node_modules/@types/node/module.d.ts:108:13 - (TS2386) Overload signatures must all be optional or required.

I can't find a way to fix it.

…to pnpm-dedupe

## Description pnpm dedupe in all packages. This can almost be done with: `pnpm exec flub exec "pnpm dedupe --ignore-scripts" --releaseGroupRoot=all --packages` The "server" release group had several issues (see #25559) so it has been reverted to be handled seperatly. Also updates @octokit/core and @octokit/rest to the latest versions to fix a compile error that happened after dedupe.

…to pnpm-dedupe

pnpm dedupe

d6f1b12

github-actions bot added area: build Build related issues area: server Server related issues (routerlicious) dependencies Pull requests that update a dependency file base: main PRs targeted against main branch labels Sep 26, 2025

CraigMacomber marked this pull request as ready for review September 26, 2025 00:43

Copilot AI review requested due to automatic review settings September 26, 2025 00:43

Copilot AI reviewed Sep 26, 2025

View reviewed changes

CraigMacomber added 2 commits September 25, 2025 20:17

Updating @octokit/core and @octokit/rest to fix compile error

f88be53

New API extractor does not include imports in API report

fbaf73d

github-actions bot added the public api change Changes to a public API label Sep 26, 2025

Override @types/node to ~20

76f2f47

CraigMacomber commented Sep 26, 2025

View reviewed changes

CraigMacomber added 2 commits September 26, 2025 10:52

Update "@types/node" to match override

5727146

Merge branch 'main' of https://github.com/microsoft/FluidFramework in…

536d811

…to pnpm-dedupe

CraigMacomber marked this pull request as draft September 26, 2025 17:53

CraigMacomber mentioned this pull request Sep 26, 2025

pnpm dedupe #25564

Merged

Merge branch 'main' of https://github.com/microsoft/FluidFramework in…

9c9c81f

…to pnpm-dedupe

github-actions bot removed area: build Build related issues dependencies Pull requests that update a dependency file labels Sep 26, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

pnpm dedupe #25559

pnpm dedupe #25559

CraigMacomber commented Sep 26, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 26, 2025

Uh oh!

Copilot AI Sep 26, 2025

Uh oh!

CraigMacomber commented Sep 26, 2025

Uh oh!

CraigMacomber Sep 26, 2025

Uh oh!

CraigMacomber Sep 26, 2025

Uh oh!

CraigMacomber Sep 26, 2025

Uh oh!

CraigMacomber Sep 26, 2025

Uh oh!

Uh oh!

-+            // BUG MITIGATION: Always check release tag compatibility.
++            // BUG MITIGATION: Previously, release tag compatibility was only checked when collectorEntity.consumable was true.
++            // This led to missed analyzer issues when entities were not marked as consumable, causing incorrect API surface validation.
++            // To ensure all relevant symbols are checked for release tag compatibility, this block always performs the comparison,
++            // regardless of the consumable status. This mitigates bugs where incompatible release tags were not reported.

-+        // PATCH: Don't emit imports
++        // PATCH: Don't emit imports.
++        // This patch disables emitting import statements in the generated API report.
++        // Rationale: Emitting imports can cause issues with downstream consumers of the API report,
++        // such as breaking type resolution or introducing unwanted dependencies. By omitting imports,
++        // we ensure the API report remains self-contained and compatible with our tooling and usage patterns.

pnpm dedupe #25559

Are you sure you want to change the base?

pnpm dedupe #25559

Conversation

CraigMacomber commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reviewer Guidance

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

CraigMacomber commented Sep 26, 2025

Uh oh!

CraigMacomber Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

CraigMacomber Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

CraigMacomber Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

CraigMacomber Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CraigMacomber commented Sep 26, 2025 •

edited

Loading