[rc3] Make ./internal/test exports conditional (microsoft#20828)

## Description Adjusts the /internal/test exports of merge-tree and id-compressor to be conditional. Cherry-pick of microsoft#20729 into rc3. This also includes fixes to two follow-up issues this commit introduced, i.e. cherry-picks microsoft#20808, and microsoft#20827. --------- Co-authored-by: Abram Sanderson <absander@microsoft.com> Co-authored-by: Alex Villarreal <716334+alexvy86@users.noreply.github.com>
anthony-murphy · Apr 26, 2024 · ee2267c · ee2267c
1 parent 9fc4a71
commit ee2267c
Show file tree

Hide file tree

Showing 17 changed files with 105 additions and 21 deletions.
diff --git a/common/build/build-common/README.md b/common/build/build-common/README.md
@@ -36,7 +36,7 @@ files](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-0
 -   tsconfig.node16.json - This config extends base and sets `module: Node16` and `moduleResolution: Node16`. It is intended for all
     builds.
 -   tsconfig.test.node16.json - This config disables some settings that we don't want to use in test code, like `declaration` and
-    `decarationMap`. It also enables the `node` types by default.
+    `decarationMap`. It also enables the `node` types by default, and turns on the "allow-ff-test-exports" [condition](https://nodejs.org/api/packages.html#conditional-exports), which allows imports for test-only indexes used in a few packages.
 
 ### Dual Build Pattern
 

diff --git a/common/build/build-common/tsconfig.test.node16.json b/common/build/build-common/tsconfig.test.node16.json
@@ -2,6 +2,7 @@
 	"extends": ["./tsconfig.base.json", "./tsconfig.node16.json"],
 	"compilerOptions": {
 		"composite": false,
+		"customConditions": ["allow-ff-test-exports"],
 		"types": ["node"],
 		"declaration": false,
 		"declarationMap": false,

diff --git a/packages/dds/merge-tree/package.json b/packages/dds/merge-tree/package.json
@@ -44,13 +44,15 @@
 			}
 		},
 		"./internal/test": {
-			"import": {
-				"types": "./lib/test/index.d.ts",
-				"default": "./lib/test/index.js"
-			},
-			"require": {
-				"types": "./dist/test/index.d.ts",
-				"default": "./dist/test/index.js"
+			"allow-ff-test-exports": {
+				"import": {
+					"types": "./lib/test/index.d.ts",
+					"default": "./lib/test/index.js"
+				},
+				"require": {
+					"types": "./dist/test/index.d.ts",
+					"default": "./dist/test/index.js"
+				}
 			}
 		}
 	},

diff --git a/packages/dds/sequence/.eslintrc.cjs b/packages/dds/sequence/.eslintrc.cjs
@@ -15,4 +15,33 @@ module.exports = {
 		"@typescript-eslint/no-use-before-define": "off",
 		"@typescript-eslint/strict-boolean-expressions": "off",
 	},
+	settings: {
+		"import/extensions": [".ts", ".tsx", ".d.ts", ".js", ".jsx"],
+		"import/parsers": {
+			"@typescript-eslint/parser": [".ts", ".tsx", ".d.ts"],
+		},
+		"import/resolver": {
+			typescript: {
+				extensions: [".ts", ".tsx", ".d.ts", ".js", ".jsx"],
+				conditionNames: [
+					"allow-ff-test-exports",
+
+					// Default condition names below, see https://www.npmjs.com/package/eslint-import-resolver-typescript#conditionnames
+					"types",
+					"import",
+
+					// APF: https://angular.io/guide/angular-package-format
+					"esm2020",
+					"es2020",
+					"es2015",
+
+					"require",
+					"node",
+					"node-addons",
+					"browser",
+					"default",
+				],
+			},
+		},
+	},
 };
diff --git a/packages/dds/sequence/src/sequence.ts b/packages/dds/sequence/src/sequence.ts
@@ -17,7 +17,9 @@ import {
 	IJSONSegment,
 	IMergeTreeAnnotateMsg,
 	IMergeTreeDeltaOp,
+	// eslint-disable-next-line import/no-deprecated
 	IMergeTreeGroupMsg,
+	// eslint-disable-next-line import/no-deprecated
 	IMergeTreeObliterateMsg,
 	IMergeTreeOp,
 	IMergeTreeRemoveMsg,
@@ -30,9 +32,11 @@ import {
 	PropertySet,
 	ReferencePosition,
 	ReferenceType,
+	// eslint-disable-next-line import/no-deprecated
 	SegmentGroup,
 	SlidingPreference,
-	createAnnotateRangeOp, // eslint-disable-next-line import/no-deprecated
+	createAnnotateRangeOp,
+	// eslint-disable-next-line import/no-deprecated
 	createGroupOp,
 	createInsertOp,
 	createObliterateRangeOp,
@@ -180,6 +184,7 @@ export abstract class SharedSegmentSequence<T extends ISegment>
 				}
 
 				case MergeTreeDeltaType.OBLITERATE: {
+					// eslint-disable-next-line import/no-deprecated
 					const lastRem = ops[ops.length - 1] as IMergeTreeObliterateMsg;
 					if (lastRem?.pos1 === r.position) {
 						assert(
@@ -243,6 +248,7 @@ export abstract class SharedSegmentSequence<T extends ISegment>
 	/** `Deferred` that triggers once the object is loaded */
 	protected loadedDeferred = new Deferred<void>();
 	// cache out going ops created when partial loading
+	// eslint-disable-next-line import/no-deprecated
 	private readonly loadedDeferredOutgoingOps: [IMergeTreeOp, SegmentGroup | SegmentGroup[]][] =
 		[];
 	// cache incoming ops that arrive when partial loading
@@ -340,6 +346,7 @@ export abstract class SharedSegmentSequence<T extends ISegment>
 	 * release, as group ops are redundant with the native batching capabilities
 	 * of the runtime
 	 */
+	// eslint-disable-next-line import/no-deprecated
 	public groupOperation(groupOp: IMergeTreeGroupMsg) {
 		this.guardReentrancy(() => this.client.localTransaction(groupOp));
 	}
@@ -654,6 +661,7 @@ export abstract class SharedSegmentSequence<T extends ISegment>
 				this.submitSequenceMessage(
 					this.client.regeneratePendingOp(
 						content as IMergeTreeOp,
+						// eslint-disable-next-line import/no-deprecated
 						localOpMetadata as SegmentGroup | SegmentGroup[],
 					),
 				);

diff --git a/packages/dds/sequence/src/sharedString.ts b/packages/dds/sequence/src/sharedString.ts
@@ -5,6 +5,7 @@
 
 import { IChannelAttributes, IFluidDataStoreRuntime } from "@fluidframework/datastore-definitions";
 import {
+	// eslint-disable-next-line import/no-deprecated
 	IMergeTreeTextHelper,
 	IRelativePosition,
 	ISegment,
@@ -87,6 +88,7 @@ export class SharedString
 		return this;
 	}
 
+	// eslint-disable-next-line import/no-deprecated
 	private readonly mergeTreeTextHelper: IMergeTreeTextHelper;
 
 	constructor(

diff --git a/packages/dds/sequence/src/test/generateSharedStrings.ts b/packages/dds/sequence/src/test/generateSharedStrings.ts
@@ -3,7 +3,6 @@
  * Licensed under the MIT License.
  */
 
-// eslint-disable-next-line import/no-internal-modules
 import { SnapshotLegacy as Snapshot } from "@fluidframework/merge-tree/internal/test";
 import * as mocks from "@fluidframework/test-runtime-utils/internal";
 import { MersenneTwister19937, Random } from "random-js";

diff --git a/packages/dds/sequence/src/test/intervalRebasing.spec.ts b/packages/dds/sequence/src/test/intervalRebasing.spec.ts
@@ -6,7 +6,6 @@
 import { strict as assert } from "assert";
 
 import { IChannelServices } from "@fluidframework/datastore-definitions";
-// eslint-disable-next-line import/no-internal-modules
 import { useStrictPartialLengthChecks } from "@fluidframework/merge-tree/internal/test";
 import {
 	MockContainerRuntimeFactoryForReconnection,

diff --git a/packages/dds/sequence/src/test/sequenceDeltaEvent.spec.ts b/packages/dds/sequence/src/test/sequenceDeltaEvent.spec.ts
@@ -14,7 +14,6 @@ import {
 	createInsertSegmentOp,
 	createRemoveRangeOp,
 } from "@fluidframework/merge-tree/internal";
-// eslint-disable-next-line import/no-internal-modules
 import { TestClient } from "@fluidframework/merge-tree/internal/test";
 
 import { SequenceDeltaEvent } from "../sequenceDeltaEvent.js";

diff --git a/packages/dds/sequence/src/test/subSequence.spec.ts b/packages/dds/sequence/src/test/subSequence.spec.ts
@@ -10,7 +10,6 @@ import {
 	createInsertSegmentOp,
 	createRemoveRangeOp,
 } from "@fluidframework/merge-tree/internal";
-// eslint-disable-next-line import/no-internal-modules
 import { TestClient } from "@fluidframework/merge-tree/internal/test";
 
 import { SubSequence } from "../sharedSequence.js";

diff --git a/packages/dds/tree/.eslintrc.cjs b/packages/dds/tree/.eslintrc.cjs
@@ -39,4 +39,33 @@ module.exports = {
 			},
 		},
 	],
+	settings: {
+		"import/extensions": [".ts", ".tsx", ".d.ts", ".js", ".jsx"],
+		"import/parsers": {
+			"@typescript-eslint/parser": [".ts", ".tsx", ".d.ts"],
+		},
+		"import/resolver": {
+			typescript: {
+				extensions: [".ts", ".tsx", ".d.ts", ".js", ".jsx"],
+				conditionNames: [
+					"allow-ff-test-exports",
+
+					// Default condition names below, see https://www.npmjs.com/package/eslint-import-resolver-typescript#conditionnames
+					"types",
+					"import",
+
+					// APF: https://angular.io/guide/angular-package-format
+					"esm2020",
+					"es2020",
+					"es2015",
+
+					"require",
+					"node",
+					"node-addons",
+					"browser",
+					"default",
+				],
+			},
+		},
+	},
 };
diff --git a/packages/dds/tree/.vscode/settings.json b/packages/dds/tree/.vscode/settings.json
@@ -12,5 +12,8 @@
 	],
 	"mochaExplorer.configFile": ".mocharc.cjs",
 	"mochaExplorer.timeout": 999999,
+	// This extension appears to invoke mocha programmatically, meaning that the enablement of this option in the common
+	// mocha test config isn't sufficient; it also needs to be enabled here.
+	"mochaExplorer.nodeArgv": ["--conditions", "allow-ff-test-exports"],
 	"cSpell.words": ["deprioritized", "endregion", "insertable", "reentrantly", "unhydrated"],
 }
diff --git a/packages/dds/tree/src/test/tsconfig.json b/packages/dds/tree/src/test/tsconfig.json
@@ -10,6 +10,7 @@
 		"module": "Node16",
 		"moduleResolution": "Node16",
 		"types": ["node", "mocha"],
+		"customConditions": ["allow-ff-test-exports"],
 	},
 	"include": ["./**/*"],
 	"references": [

diff --git a/packages/runtime/id-compressor/package.json b/packages/runtime/id-compressor/package.json
@@ -44,13 +44,15 @@
 			}
 		},
 		"./internal/test-utils": {
-			"import": {
-				"types": "./lib/test/index.d.ts",
-				"default": "./lib/test/index.js"
-			},
-			"require": {
-				"types": "./dist/test/index.d.ts",
-				"default": "./dist/test/index.js"
+			"allow-ff-test-exports": {
+				"import": {
+					"types": "./lib/test/index.d.ts",
+					"default": "./lib/test/index.js"
+				},
+				"require": {
+					"types": "./dist/test/index.d.ts",
+					"default": "./dist/test/index.js"
+				}
 			}
 		}
 	},

diff --git a/packages/test/mocha-test-setup/mocharc-common.cjs b/packages/test/mocha-test-setup/mocharc-common.cjs
@@ -51,6 +51,8 @@ function getFluidTestMochaConfig(packageDir, additionalRequiredModules, testRepo
 		"recursive": true,
 		"require": requiredModulePaths,
 		"unhandled-rejections": "strict",
+		// Allow test-only indexes to be imported. Search the FF repo for package.json files with this condition to see example usage.
+		"node-option": "conditions=allow-ff-test-exports",
 		// Performance tests benefit from having access to GC, and memory tests require it.
 		// Exposing it here avoids all packages which do perf testing from having to expose it.
 		"v8-expose-gc": true,

diff --git a/packages/test/test-end-to-end-tests/package.json b/packages/test/test-end-to-end-tests/package.json
@@ -42,7 +42,7 @@
 		"test:realsvc:local:report:full": "cross-env fluid__test__backCompat=FULL npm run test:realsvc:local --",
 		"test:realsvc:odsp": "npm run test:realsvc:run -- --driver=odsp --timeout=20s",
 		"test:realsvc:odsp:report": "npm run test:realsvc:odsp --",
-		"test:realsvc:r11s": "npm run test:realsvc:run -- --driver=r11s --timeout=20s --use-openssl-ca",
+		"test:realsvc:r11s": "npm run test:realsvc:run -- --driver=r11s --timeout=20s",
 		"test:realsvc:r11s:docker": "npm run test:realsvc:run -- --driver=r11s --r11sEndpointName=docker --timeout=20s",
 		"test:realsvc:routerlicious": "npm run test:realsvc:r11s",
 		"test:realsvc:routerlicious:report": "npm run test:realsvc:r11s --",

diff --git a/packages/test/test-end-to-end-tests/src/test/.mocharc.cjs b/packages/test/test-end-to-end-tests/src/test/.mocharc.cjs
@@ -8,4 +8,13 @@
 const packageDir = `${__dirname}/../..`;
 const getFluidTestMochaConfig = require("@fluid-private/test-version-utils/mocharc-common");
 const config = getFluidTestMochaConfig(packageDir);
+
+// The 'use-openssl-ca' node option used to be passed as a flag in the npm script in package.json, but
+// adding node-option to the base mocharc-common.cjs caused it to be ignored, so we need to append it here.
+if (config["node-option"] === undefined) {
+	config["node-option"] = "use-openssl-ca";
+} else {
+	config["node-option"] += ",use-openssl-ca";
+}
+
 module.exports = config;