Skip to content
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

fix(client-utils): fix node/browser dependent support #21321

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions packages/common/client-utils/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 -->
Expand Down
16 changes: 16 additions & 0 deletions packages/common/client-utils/bufferBrowser.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*!
* Copyright (c) Microsoft Corporation and contributors. All rights reserved.
* Licensed under the MIT License.
*/

/*
* THIS FILE SHOULD NOT BE DELETED.
*
* This file is needed for compatibility with TypeScript's Node10 module resolution. In most other packages it is
* auto-generated by `flub generate entrypoints` in @fluid-tools/build-cli, and is thus gitignored in most packages.
*
* However, this package is unique in that it does not need to use API trimming or `flub generate entrypoints`,
* but instead uses conditional exports to isomorphically support both Node and browser environments.
scottn12 marked this conversation as resolved.
Show resolved Hide resolved
*/

export * from "./lib/bufferBrowser.js";
36 changes: 21 additions & 15 deletions packages/common/client-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,30 @@
"type": "module",
"exports": {
".": {
"import": {
"types": "./lib/indexNode.d.ts",
"default": "./lib/indexNode.js"
"node": {
"import": {
"types": "./lib/indexNode.d.ts",
"default": "./lib/indexNode.js"
},
"require": {
"types": "./dist/indexNode.d.ts",
"default": "./dist/indexNode.js"
}
},
"require": {
"types": "./dist/indexNode.d.ts",
"default": "./dist/indexNode.js"
"default": {
"import": {
"types": "./lib/indexBrowser.d.ts",
"default": "./lib/indexBrowser.js"
},
"require": {
"types": "./dist/indexBrowser.d.ts",
"default": "./dist/indexBrowser.js"
}
}
}
},
"main": "dist/indexNode.js",
"browser": {
"./dist/indexNode.js": "./dist/indexBrowser.js",
"./dist/indexNode.d.ts": "./dist/indexBrowser.d.ts",
"./lib/indexNode.js": "./lib/indexBrowser.js",
"./lib/indexNode.d.ts": "./lib/indexBrowser.d.ts"
},
"types": "dist/indexNode.d.ts",
"main": "lib/indexBrowser.js",
"types": "lib/indexBrowser.d.ts",
"scripts": {
"api": "fluid-build . --task api",
"api-extractor:commonjs": "api-extractor run --config ./api-extractor-cjs.json",
Expand Down Expand Up @@ -64,7 +70,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": "fluid-type-test-generator",
"typetests:prepare": "flub typetests --dir . --reset --previous --normalize"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
* Licensed under the MIT License.
*/

import { IsoBuffer } from "./indexNode.js";
// 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";

/**
* Converts the provided {@link https://en.wikipedia.org/wiki/Base64 | base64}-encoded string
Expand Down
50 changes: 50 additions & 0 deletions packages/common/client-utils/src/base64EncodingNode.ts
Original file line number Diff line number Diff line change
@@ -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();
}
}
};
7 changes: 0 additions & 7 deletions packages/common/client-utils/src/cjs/package.json

This file was deleted.

2 changes: 2 additions & 0 deletions packages/common/client-utils/src/hashFileBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*/

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> {
Expand Down
1 change: 1 addition & 0 deletions packages/common/client-utils/src/hashFileNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

/**
Expand Down
5 changes: 4 additions & 1 deletion packages/common/client-utils/src/indexBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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";
Expand Down
5 changes: 4 additions & 1 deletion packages/common/client-utils/src/indexNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
24 changes: 17 additions & 7 deletions packages/common/client-utils/src/test/jest/buffer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ import * as BufferNode from "../../bufferNode.js";
import * as BufferBrowser from "../../bufferBrowser.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 = [
"",
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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");
});
});
17 changes: 17 additions & 0 deletions packages/common/client-utils/src/test/mocha/base64Encoding.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
22 changes: 22 additions & 0 deletions packages/common/client-utils/src/test/mocha/buffer.spec.ts
Original file line number Diff line number Diff line change
@@ -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.",
);
});
});
34 changes: 34 additions & 0 deletions packages/common/client-utils/src/test/mocha/trace.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
2 changes: 1 addition & 1 deletion packages/common/client-utils/src/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading