fix(client-utils): fix trace.js import of performance (#21257)

packages/common/client-utils/README.md

@@ -36,6 +36,29 @@ If you want to add code that does not meet these requirements, these other packa
     **core-interfaces** package.
 -   **Zero-dependency shared code** should be put in the **core-utils** package.
 
+## Isomorphic Code
+
+One of the primary reasons for this package's existence is to provide isomorphic implementations of
+Buffer and related utilities that work in both browser and Node.js environments.
+
+Our general strategy for this is as follows:
+
+-   We use the export map in package.json to provide different entrypoints for browser (indexBrowser.js)
+    vs. Node.js (indexNode.js).
+
+-   Because the browser ecosystem is more complex (bunders, etc.), we improve our odds of success by making
+    the browser the default. Only Node.js relies on remapping via the export map.
+
+-   We further simplify things by only using the export map to resolve the initial entrypoint. We do not
+    rely on export maps to remap imports within the module. (Basically, the browser / node.js specific
+    implementations fork at the entrypoint and from that point on explicitly import browser or node
+    specific files.)
+
+One thing it is important to be aware of is that our CJS support relies on copying a stub package.json
+file to dist/package.json to set the module type to commonjs. When resolving internal imports for CJS
+packages, module resolution will walk up from the \*.js file and discover this stub package.json. Because
+the stub package.json lacks an export map, internal imports will not be remapped.
+
 <!-- AUTO-GENERATED-CONTENT:START (README_DEPENDENCY_GUIDELINES_SECTION:includeHeading=TRUE) -->
 
 <!-- prettier-ignore-start -->

packages/common/client-utils/package.json

@@ -34,32 +34,10 @@
 					"default": "./dist/indexBrowser.js"
 				}
 			}
-		},
-		"./bufferBrowser.js": {
-			"node": {
-				"import": {
-					"types": "./lib/bufferNode.d.ts",
-					"default": "./lib/bufferNode.js"
-				},
-				"require": {
-					"types": "./dist/bufferNode.d.ts",
-					"default": "./dist/bufferNode.js"
-				}
-			},
-			"default": {
-				"import": {
-					"types": "./lib/bufferBrowser.d.ts",
-					"default": "./lib/bufferBrowser.js"
-				},
-				"require": {
-					"types": "./dist/bufferBrowser.d.ts",
-					"default": "./dist/bufferBrowser.js"
-				}
-			}
 		}
 	},
-	"main": "lib/indexNode.js",
-	"types": "lib/indexNode.d.ts",
+	"main": "lib/indexBrowser.js",
+	"types": "lib/indexBrowser.d.ts",
 	"scripts": {
 		"build": "fluid-build . --task build",
 		"build:commonjs": "fluid-build . --task commonjs",
@@ -89,7 +67,7 @@
 		"test:mocha": "npm run test:mocha:esm && npm run test:mocha:cjs",
 		"test:mocha:cjs": "mocha --recursive \"dist/test/mocha/**/*.spec.*js\" --exit -r node_modules/@fluid-internal/mocha-test-setup",
 		"test:mocha:esm": "mocha --recursive \"lib/test/mocha/**/*.spec.*js\" --exit -r node_modules/@fluid-internal/mocha-test-setup",
-		"tsc": "fluid-tsc commonjs --project ./tsconfig.cjs.json && copyfiles -f ./src/cjs/package.json ./dist",
+		"tsc": "fluid-tsc commonjs --project ./tsconfig.cjs.json && copyfiles -f ../../../common/build/build-common/src/cjs/package.json ./dist",
 		"typetests:gen": "flub generate typetests --dir . -v --level public",
 		"typetests:prepare": "flub typetests --dir . --reset --previous --normalize"
 	},

packages/common/client-utils/src/base64Encoding.ts → packages/common/client-utils/src/base64EncodingBrowser.ts

@@ -3,7 +3,11 @@
  * Licensed under the MIT License.
  */
 
-// Note: The exports map in package.json will correct this import to "./bufferNode.js" in node environments
+// This file is a Node.js-specific implementation of the base64 encoding functions.
+// Aside from the below import statement, this file should be identical to the
+// base64EncodingNode.ts.
+//
+// (See 'Isomorphic Code' section in the package README.md.)
 import { IsoBuffer } from "./bufferBrowser.js";
 
 /**

packages/common/client-utils/src/base64EncodingNode.ts

@@ -0,0 +1,50 @@
+/*!
+ * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+// This file is a Node.js-specific implementation of the base64 encoding functions.
+// Aside from the below import statement, this file should be identical to the
+// base64EncodingBrowser.ts.
+//
+// (See 'Isomorphic Code' section in the package README.md.)
+import { IsoBuffer } from "./bufferNode.js";
+
+/**
+ * Converts the provided {@link https://en.wikipedia.org/wiki/Base64 | base64}-encoded string
+ * to {@link https://en.wikipedia.org/wiki/UTF-8 | utf-8}.
+ *
+ * @internal
+ */
+export const fromBase64ToUtf8 = (input: string): string =>
+	IsoBuffer.from(input, "base64").toString("utf8");
+
+/**
+ * Converts the provided {@link https://en.wikipedia.org/wiki/UTF-8 | utf-8}-encoded string
+ * to {@link https://en.wikipedia.org/wiki/Base64 | base64}.
+ *
+ * @internal
+ */
+export const fromUtf8ToBase64 = (input: string): string =>
+	IsoBuffer.from(input, "utf8").toString("base64");
+
+/**
+ * Convenience function to convert unknown encoding to utf8 that avoids
+ * buffer copies/encode ops when no conversion is needed.
+ * @param input - The source string to convert.
+ * @param encoding - The source string's encoding.
+ *
+ * @internal
+ */
+export const toUtf8 = (input: string, encoding: string): string => {
+	switch (encoding) {
+		case "utf8":
+		// eslint-disable-next-line unicorn/text-encoding-identifier-case -- this value is supported, just discouraged
+		case "utf-8": {
+			return input;
+		}
+		default: {
+			return IsoBuffer.from(input, encoding).toString();
+		}
+	}
+};

packages/common/client-utils/src/hashFileBrowser.ts

@@ -5,6 +5,7 @@
 
 import * as base64js from "base64-js";
 
+// Note: See 'Isomorphic Code' section in the package README.md
 import { IsoBuffer } from "./bufferBrowser.js";
 
 async function digestBuffer(file: IsoBuffer, algorithm: "SHA-1" | "SHA-256"): Promise<Uint8Array> {

packages/common/client-utils/src/hashFileNode.ts

@@ -5,6 +5,7 @@
 
 import { sha1, sha256 } from "sha.js";
 
+// Note: See 'Isomorphic Code' section in the package README.md
 import type { IsoBuffer } from "./bufferNode.js";
 
 /**

packages/common/client-utils/src/indexBrowser.ts

@@ -3,6 +3,9 @@
  * Licensed under the MIT License.
  */
 
+// Entrypoint for browser-specific code in the package.
+// (See 'Isomorphic Code' section in the package README.md.)
+
 export {
 	bufferToString,
 	isArrayBuffer,
@@ -13,7 +16,7 @@ export {
 export { gitHashFile, hashFile } from "./hashFileBrowser.js";
 export { performance } from "./performanceIsomorphic.js";
 
-export { fromBase64ToUtf8, fromUtf8ToBase64, toUtf8 } from "./base64Encoding.js";
+export { fromBase64ToUtf8, fromUtf8ToBase64, toUtf8 } from "./base64EncodingBrowser.js";
 export { Uint8ArrayToArrayBuffer } from "./bufferShared.js";
 export { EventEmitter } from "./eventEmitter.cjs";
 export { type IsomorphicPerformance } from "./performanceIsomorphic.js";

packages/common/client-utils/src/indexNode.ts

@@ -3,12 +3,15 @@
  * Licensed under the MIT License.
  */
 
+// Entrypoint for Node.js-specific code in the package.
+// (See 'Isomorphic Code' section in the package README.md.)
+
 export { type Buffer } from "./bufferNode.js";
 export { bufferToString, IsoBuffer, stringToBuffer, Uint8ArrayToString } from "./bufferNode.js";
 export { gitHashFile, hashFile } from "./hashFileNode.js";
 export { performance } from "./performanceIsomorphic.js";
 
-export { fromBase64ToUtf8, fromUtf8ToBase64, toUtf8 } from "./base64Encoding.js";
+export { fromBase64ToUtf8, fromUtf8ToBase64, toUtf8 } from "./base64EncodingNode.js";
 export { Uint8ArrayToArrayBuffer } from "./bufferShared.js";
 export { EventEmitter } from "./eventEmitter.cjs";
 export { IsomorphicPerformance } from "./performanceIsomorphic.js";

packages/common/client-utils/src/test/jest/buffer.spec.ts

@@ -7,6 +7,16 @@ import * as BufferBrowser from "../../bufferBrowser.js";
 import * as BufferNode from "../../bufferNode.js";
 
 describe("Buffer isomorphism", () => {
+	test("returns the expected implementation", () => {
+		// BufferNode should create a native Node.js Buffer instance
+		const nodeBuffer = BufferNode.IsoBuffer.from("", "utf8");
+		expect(nodeBuffer.constructor.name).toEqual("Buffer");
+
+		// BufferBrowser should create our partial Buffer polyfil.
+		const browserBuffer = BufferBrowser.IsoBuffer.from("", "utf8");
+		expect(browserBuffer.constructor.name).toEqual("IsoBuffer");
+	});
+
 	test("from string utf-8/16 is compatible", () => {
 		const testArray = [
 			"",
@@ -135,7 +145,7 @@ describe("Buffer isomorphism", () => {
 
 			const encodedWithoutView = new TextEncoder().encode(item).buffer;
 			const nodeBufferWithoutView = BufferNode.IsoBuffer.from(encodedWithoutView);
-			const browserBufferWithoutView = BufferNode.IsoBuffer.from(encodedWithoutView);
+			const browserBufferWithoutView = BufferBrowser.IsoBuffer.from(encodedWithoutView);
 
 			expect(nodeBufferWithoutView.toString("base64")).toEqual(nodeBuffer.toString("base64"));
 			expect(browserBufferWithoutView.toString("base64")).toEqual(
@@ -253,14 +263,14 @@ describe("Buffer isomorphism", () => {
 	test("bufferToString working with IsoBuffer", () => {
 		const test = "aGVsbG90aGVyZQ==";
 
-		const buffer = BufferBrowser.IsoBuffer.from(test, "base64");
-		expect(BufferBrowser.bufferToString(buffer, "base64")).toEqual(test);
+		const browserBuffer = BufferBrowser.IsoBuffer.from(test, "base64");
+		expect(BufferBrowser.bufferToString(browserBuffer, "base64")).toEqual(test);
 		// eslint-disable-next-line unicorn/text-encoding-identifier-case -- this value is supported, just discouraged
-		expect(BufferBrowser.bufferToString(buffer, "utf-8")).toEqual("hellothere");
+		expect(BufferBrowser.bufferToString(browserBuffer, "utf-8")).toEqual("hellothere");
 
-		const buffer2 = BufferNode.IsoBuffer.from(test, "base64");
-		expect(BufferNode.bufferToString(buffer2, "base64")).toEqual(test);
+		const nodeBuffer = BufferNode.IsoBuffer.from(test, "base64");
+		expect(BufferNode.bufferToString(nodeBuffer, "base64")).toEqual(test);
 		// eslint-disable-next-line unicorn/text-encoding-identifier-case -- this value is supported, just discouraged
-		expect(BufferNode.bufferToString(buffer2, "utf-8")).toEqual("hellothere");
+		expect(BufferNode.bufferToString(nodeBuffer, "utf-8")).toEqual("hellothere");
 	});
 });

packages/common/client-utils/src/test/mocha/base64Encoding.spec.ts

@@ -0,0 +1,17 @@
+/*!
+ * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+import { strict as assert } from "node:assert";
+
+import { fromBase64ToUtf8, fromUtf8ToBase64, toUtf8 } from "../../indexNode.js";
+
+describe("base64Encoding", () => {
+	it("round-trips correctly", async () => {
+		const original = "hello world";
+		const base64 = fromUtf8ToBase64(original);
+		assert.equal(fromBase64ToUtf8(base64), original);
+		assert.equal(toUtf8(base64, "base64"), original);
+	});
+});

packages/common/client-utils/src/test/mocha/buffer.spec.ts

@@ -0,0 +1,22 @@
+/*!
+ * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+import { strict as assert } from "node:assert";
+
+import { IsoBuffer } from "../../indexNode.js";
+
+describe("IsoBuffer", () => {
+	it("is a Buffer", () => {
+		// Paranoid test that 'IsoBuffer' can be constructed in both CJS/ESM in Node.js environments.
+		// Note that no export remapping is involved, given that 'IsoBuffer' is imported from 'indexNode.js'.
+		//
+		// More comprehensive testing for 'IsoBuffer' is done in 'jest/buffer.spec.ts', which compares
+		// Node.js and Broswer implementations for equivalence.
+		assert(
+			IsoBuffer.from("", "utf8") instanceof Buffer,
+			"Expected IsoBuffer.from to return a native Node.js Buffer instance.",
+		);
+	});
+});

packages/common/client-utils/src/test/mocha/trace.spec.ts

@@ -0,0 +1,34 @@
+/*!
+ * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+import { strict as assert } from "node:assert";
+
+import { Trace } from "../../indexNode.js";
+
+describe("Trace", () => {
+	it("measure 2ms timeout", async () => {
+		const trace = Trace.start();
+
+		return new Promise((resolve, reject) => {
+			setTimeout(() => {
+				try {
+					const event = trace.trace();
+
+					// While we specify a 2ms timeout, it's possible for performance.now() to measure a
+					// slightly smaller duration due to timing attack and fingerprinting mitigations.
+					// (See https://developer.mozilla.org/en-US/docs/Web/API/Performance/now#security_requirements)
+					//
+					// Therefore, we conservatively require that only 1ms has elapsed.
+					assert(event.duration >= 1);
+					assert(event.totalTimeElapsed >= 1);
+
+					resolve(undefined);
+				} catch (error) {
+					reject(error);
+				}
+			}, 2);
+		});
+	});
+});

packages/common/client-utils/src/trace.ts

@@ -3,7 +3,7 @@
  * Licensed under the MIT License.
  */
 
-import { performance } from "./indexNode.js";
+import { performance } from "./performanceIsomorphic.js";
 
 /**
  * Helper class for tracing performance of events