Updated eslint config to latest for packages/loader/driver-utils #24905

krauders · 2025-06-24T18:20:53Z

Description

Updated packages/loader/driver-utils to latest eslint configuration and resolved all issues, except for a few that were disabled with per-line comments because they required deeper changes.

Copilot

Pull Request Overview

This PR updates the ESLint configuration for packages/loader/driver-utils and resolves various linting issues. Key changes include refining type annotations (e.g. using unknown instead of any), replacing deprecated string methods (substr to slice), and improving explicit return types and error handling across source and test files.

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/loader/driver-utils/src/treeConversions.ts	Added ESLint disable comment and adjusted switch-case blocks.
packages/loader/driver-utils/src/test/summaryCompresssionTester.spec.ts	Applied explicit typings and minor improvements in string handling.
packages/loader/driver-utils/src/test/runWithRetry.spec.ts	Updated error types and refined async function signatures.
packages/loader/driver-utils/src/test/rateLimiter.spec.ts	Updated import for assert from "node:assert".
packages/loader/driver-utils/src/test/parallelRequests.spec.ts	Introduced explicit return types and adjusted payload length checks.
packages/loader/driver-utils/src/test/insecureUrlResolverTest.spec.ts	Updated assert import to "node:assert".
packages/loader/driver-utils/src/runWithRetry.ts	Changed error type to unknown and updated delay calculation.
packages/loader/driver-utils/src/rateLimiter.ts	Added explicit return types for methods.
packages/loader/driver-utils/src/protocol/gitHelper.ts	Updated switch-case braces for consistency.
packages/loader/driver-utils/src/prefetchDocumentStorageService.ts	Refined returned types and added ESLint disable comments.
packages/loader/driver-utils/src/parallelRequests.ts	Improved type annotations and control flow assertions.
packages/loader/driver-utils/src/networkUtils.ts	Adjusted telemetry logging with explicit type casting for connection.
packages/loader/driver-utils/src/network.ts	Updated error property types and error delay conversion logic.
packages/loader/driver-utils/src/messageRecognition.ts	Minor documentation update.
packages/loader/driver-utils/src/insecureUrlResolver.ts	Replaced substr with slice for URL path extraction.
packages/loader/driver-utils/src/documentStorageServiceProxy.ts	Refined parameter types for getSnapshotTree and getVersions.
packages/loader/driver-utils/src/documentServiceProxy.ts	Updated dispose signature to accept unknown error type.
packages/loader/driver-utils/src/buildSnapshotTree.ts	Removed FileMode conversion—ensure that treeEntry.mode still meets API needs.
packages/loader/driver-utils/src/blob.ts	Added ESLint disable comment for text encoding formatting.
packages/loader/driver-utils/src/adapters/predefinedAdapters.ts	Updated isCompressionConfig to check object type before proceeding.
packages/loader/driver-utils/src/adapters/compression/summaryblob/documentStorageServiceSummaryBlobCompressionAdapter.ts	Updated compression adapter with additional type clarity and ESLint disable comments.
packages/loader/driver-utils/.eslintrc.cjs	Updated ESLint base configuration usage.

alexvy86 · 2025-06-26T14:51:24Z

.../src/adapters/compression/summaryblob/documentStorageServiceSummaryBlobCompressionAdapter.ts

+		if (metadataHolderTree === undefined) {
+			return undefined;
+		}


This feels redundant, no? The return right below this would already result in the same thing?

alexvy86 · 2025-06-26T14:55:45Z

.../src/adapters/compression/summaryblob/documentStorageServiceSummaryBlobCompressionAdapter.ts

+		// eslint-disable-next-line unicorn/no-null
+		return snapshotTree ?? null;


Why do we need this? Seems like the typing doesn't allow snapshotTree to be undefined.

alexvy86 · 2025-06-26T14:57:28Z

packages/loader/driver-utils/src/blob.ts

@@ -29,6 +29,7 @@ export class BlobTreeEntry {
 	constructor(
 		public readonly path: string,
 		contents: string,
+		// eslint-disable-next-line unicorn/text-encoding-identifier-case


Would be nice if we can find the owner of this code to confirm why we need to stick with "utf-8'. I believe in some places it's necessary because we write it somewhere so need to be able to understand it when reading, but I don't know if that's the case too here.

alexvy86 · 2025-06-26T15:08:13Z

packages/loader/driver-utils/src/parallelRequests.ts

 			if (messages === undefined) {
 				return { done: true };
 			}
-			const value = await messages;
+			const currentMessages = messages;


Add a small comment about why do it this way? Something related to atomicity, IIRC?

alexvy86 · 2025-06-26T15:16:55Z

packages/loader/driver-utils/src/buildSnapshotTree.ts

@@ -33,7 +32,7 @@ function flattenCore(
 			blobMap.set(id, buffer);

 			const entry: IGitTreeEntry = {
-				mode: FileMode[treeEntry.mode],
+				mode: treeEntry.mode,


This seems correct since treeEntry.mode is already of type FileMode, I guess I'm just surprised the compiler didn't catch this before? I might be doing something wrong but in TS Playground I can't get something like the earlier version to even compile, and the runtime value is different in each case. Leaves me a bit concerned about what this code was actually doing and if this is a behavioral change.

alexvy86 · 2025-06-26T15:30:01Z

packages/loader/driver-utils/src/runWithRetry.ts

- * @param delayMs - wait time for previous iteration
- * @param error - error based on which we decide wait time.
- * @returns Wait time to wait for.
+ * Calculates the maximum wait time before next retry based on previous delay and error.


Any reason for the change here?

alexvy86 · 2025-06-26T15:30:58Z

packages/loader/driver-utils/src/treeConversions.ts

@@ -31,6 +31,7 @@ export function convertSummaryTreeToSnapshotITree(summaryTree: ISummaryTree): IT
 		switch (value.type) {
 			case SummaryType.Blob: {
 				let parsedContent: string;
+				// eslint-disable-next-line unicorn/text-encoding-identifier-case


Similar to a previous comment, the reason for the disable would be great.

alexvy86 · 2025-06-26T15:35:07Z

packages/loader/driver-utils/src/test/runWithRetry.spec.ts

+		} catch {
+			// Intentionally empty: error is expected in this test case
+		}


Unrelated to the change: this test doesn't seem to be testing what it says it does 🤔 . Not for this PR, but if we can see who added it maybe we should ping them to look into it.

alexvy86 · 2025-06-26T15:38:58Z

packages/loader/driver-utils/src/test/runWithRetry.spec.ts

+		} catch {
+			// Intentionally empty: error is expected in this test case
+		}


Like above, this one leaves me scratching my head. "Should not retry if what is disabled?". Then there's an onRetry that feels to me like is an error that indicates a problem with the test if it's thrown, but the empty catch might just swallow. Another one not for this PR but if we can find an owner, let's point them to it.

alexvy86 · 2025-06-26T15:40:05Z

packages/loader/driver-utils/src/test/summaryCompresssionTester.spec.ts

+			".channels"
+		] as ISummaryTree
+	).tree["7a99532d-94ec-43ac-8a53-d9f978ad4ae9"] as ISummaryTree;
+	const header: import("@fluidframework/driver-definitions").SummaryObject | undefined =


Shouldn't this import be at the top? Also seen in generateSummaryWithBinaryContent.

Josmithr · 2025-06-26T17:32:55Z

packages/loader/driver-utils/src/network.ts

+	const retryAfter = (error as { retryAfterSeconds?: unknown })?.retryAfterSeconds;
+	return retryAfter === undefined ? undefined : (retryAfter as number) * 1000;


Nit: to be safe, we should probably also check if retryAfter is actually a number. E.g.

Suggested change

const retryAfter = (error as { retryAfterSeconds?: unknown })?.retryAfterSeconds;

return retryAfter === undefined ? undefined : (retryAfter as number) * 1000;

const retryAfter = (error as { retryAfterSeconds?: unknown })?.retryAfterSeconds;

return typeof retryAfter === "number" ? retryAfter * 1000 : undefined;

Josmithr · 2025-06-26T17:36:17Z

packages/loader/driver-utils/src/networkUtils.ts

+		const nav = navigator as unknown as {
+			connection?: unknown;
+			mozConnection?: unknown;
+			webkitConnection?: unknown;
+		};
+		const connObj = nav.connection ?? nav.mozConnection ?? nav.webkitConnection;
+		if (connObj !== undefined && typeof connObj === "object") {
+			const { type } = connObj as { type?: unknown };
+			if (typeof type === "string") {
+				newEvent.connectionType = type;
+			}


Nit: general guidance is to avoid unnecessary abbreviations where possible for developer accessibility. Something like the following would be prefered:

Suggested change

const nav = navigator as unknown as {

connection?: unknown;

mozConnection?: unknown;

webkitConnection?: unknown;

};

const connObj = nav.connection ?? nav.mozConnection ?? nav.webkitConnection;

if (connObj !== undefined && typeof connObj === "object") {

const { type } = connObj as { type?: unknown };

if (typeof type === "string") {

newEvent.connectionType = type;

}

const navigatorWithConnectionProps = navigator as unknown as {

connection?: unknown;

mozConnection?: unknown;

webkitConnection?: unknown;

};

const connectionObject = nav.connection ?? nav.mozConnection ?? nav.webkitConnection;

if (connectionObject !== undefined && typeof connectionObject === "object") {

const { type } = connectionObjectas { type?: unknown };

if (typeof type === "string") {

newEvent.connectionType = type;

}

Josmithr · 2025-06-26T17:42:31Z

packages/loader/driver-utils/src/test/summaryCompresssionTester.spec.ts

+        \norigin     : ${originBlobContent as string}
+        \ndownloaded : ${downloadedBlobContent}`,


Intentional change?

krauders added 3 commits June 24, 2025 10:48

Update eslint config and resolve errors for packages/loader/driver-utils

a49cbef

Remove some unnecessary comments

2f491f7

More comment updates

e0eb0b4

Copilot AI review requested due to automatic review settings June 24, 2025 18:20

Merge branch 'main' into users/krauders/eslint-loader-driver-utils

d921ead

github-actions bot added the area: loader Loader related issues label Jun 24, 2025

Copilot AI reviewed Jun 24, 2025

View reviewed changes

github-actions bot added the base: main PRs targeted against main branch label Jun 24, 2025

Trailing comma

f1067ce

alexvy86 reviewed Jun 26, 2025

View reviewed changes

Josmithr reviewed Jun 26, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Updated eslint config to latest for packages/loader/driver-utils #24905

Updated eslint config to latest for packages/loader/driver-utils #24905

Uh oh!

krauders commented Jun 24, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

alexvy86 Jun 26, 2025

Uh oh!

alexvy86 Jun 26, 2025

Uh oh!

alexvy86 Jun 26, 2025

Uh oh!

alexvy86 Jun 26, 2025

Uh oh!

alexvy86 Jun 26, 2025

Uh oh!

alexvy86 Jun 26, 2025

Uh oh!

alexvy86 Jun 26, 2025

Uh oh!

alexvy86 Jun 26, 2025

Uh oh!

alexvy86 Jun 26, 2025

Uh oh!

alexvy86 Jun 26, 2025

Uh oh!

Josmithr Jun 26, 2025

Uh oh!

Josmithr Jun 26, 2025 •

edited

Loading

Uh oh!

Josmithr Jun 26, 2025

Uh oh!

Uh oh!

		// eslint-disable-next-line unicorn/no-null
		return snapshotTree ?? null;

		const retryAfter = (error as { retryAfterSeconds?: unknown })?.retryAfterSeconds;
		return retryAfter === undefined ? undefined : (retryAfter as number) * 1000;

		\norigin : ${originBlobContent as string}
		\ndownloaded : ${downloadedBlobContent}`,

Updated eslint config to latest for packages/loader/driver-utils #24905

Are you sure you want to change the base?

Updated eslint config to latest for packages/loader/driver-utils #24905

Uh oh!

Conversation

krauders commented Jun 24, 2025

Description

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Josmithr Jun 26, 2025 •

edited

Loading