build(merge-tree,sequence): Fix type generation for ESM and add expor…

…ts field (#18825) This PR updates sequence and merge-tree to generate proper types in ESM builds and adds an exports field. I had to update the test files a bit to clean up imports. I also added a ./dist/test export from merge-tree since its internals are used by sequence tests. This is a demonstration of the mechanism we will have to use any time we want to export something for use in another package. No more reaching into internals! This PR includes some configuration and settings changes that are not strictly needed for this PR, but splitting them out doesn't seem worth the time since they're all needed for other pending PRs like #18823 and #18824.
microsoft · Dec 14, 2023 · de43f3c · de43f3c
1 parent ed7bc19
commit de43f3c
Show file tree

Hide file tree

Showing 20 changed files with 308 additions and 120 deletions.
diff --git a/common/build/build-common/tsc-multi.test.json b/common/build/build-common/tsc-multi.test.json
@@ -1,4 +1,4 @@
 {
-	"targets": [{ "extname": ".cjs", "module": "CommonJS", "moduleResolution": "Node10" }],
+	"targets": [{ "extname": ".cjs", "module": "CommonJS", "moduleResolution": "Node16" }],
 	"projects": ["./tsconfig.json", "./src/test/tsconfig.json"]
 }
diff --git a/fluidBuild.config.cjs b/fluidBuild.config.cjs
@@ -3,7 +3,7 @@
  * Licensed under the MIT License.
  */
 
-const tscDependsOn = ["^tsc", "^api", "build:genver"];
+const tscDependsOn = ["^tsc", "^api", "^build:rename-types", "build:genver"];
 /**
  * The settings in this file configure the Fluid build tools, such as fluid-build and flub. Some settings apply to the
  * whole repo, while others apply only to the client release group.
@@ -23,7 +23,13 @@ module.exports = {
 			script: false,
 		},
 		"compile": {
-			dependsOn: ["commonjs", "build:esnext", "build:test", "build:copy"],
+			dependsOn: [
+				"commonjs",
+				"build:esnext",
+				"build:test",
+				"build:copy",
+				"build:rename-types",
+			],
 			script: false,
 		},
 		"commonjs": {
@@ -58,10 +64,11 @@ module.exports = {
 			dependsOn: ["api-extractor:commonjs", "api-extractor:esnext"],
 			script: false,
 		},
-		"api-extractor:commonjs": [...tscDependsOn, "tsc"],
-		"api-extractor:esnext": [...tscDependsOn, "api-extractor:commonjs", "build:esnext"],
-		"build:docs": [...tscDependsOn, "tsc"],
-		"ci:build:docs": [...tscDependsOn, "tsc"],
+		"api-extractor:commonjs": ["tsc"],
+		"api-extractor:esnext": ["api-extractor:commonjs", "build:esnext"],
+		"build:rename-types": ["build:esnext", "api-extractor:esnext"],
+		"build:docs": ["tsc"],
+		"ci:build:docs": ["tsc"],
 		"build:readme": {
 			dependsOn: ["build:manifest"],
 			script: true,
@@ -159,9 +166,17 @@ module.exports = {
 				// Can be removed once the policy handler is updated to support tsc-multi as equivalent to tsc.
 				"^azure/packages/azure-client/package.json",
 				"^azure/packages/azure-service-utils/package.json",
+				"^experimental/dds/tree2/package.json",
+				"^experimental/dds/sequence-deprecated/package.json",
+				"^experimental/framework/tree-react-api/package.json",
+				"^packages/common/.*/package.json",
 				"^packages/dds/.*/package.json",
 				"^packages/drivers/.*/package.json",
 				"^packages/framework/.*/package.json",
+				"^packages/loader/.*/package.json",
+				"^packages/runtime/.*/package.json",
+				"^packages/service-clients/.*/package.json",
+				"^packages/utils/.*/package.json",
 				"^packages/loader/container-loader/package.json",
 			],
 			"html-copyright-file-header": [
@@ -217,9 +232,10 @@ module.exports = {
 				"^tools/getkeys",
 			],
 			"npm-package-json-esm": [
-				// This is an ESM-only package, and uses tsc to build the ESM output. The policy handler doesn't understand this
+				// These are ESM-only packages and use tsc to build the ESM output. The policy handler doesn't understand this
 				// case.
 				"packages/dds/migration-shim/package.json",
+				"packages/test/functional-tests/package.json",
 			],
 			// This handler will be rolled out slowly, so excluding most packages here while we roll it out.
 			"npm-package-exports-field": [
@@ -315,6 +331,9 @@ module.exports = {
 				["depcruise", "dependency-cruiser"],
 				["copyfiles", "copyfiles"],
 				["oclif", "oclif"],
+				["renamer", "renamer"],
+				["tsc-multi", "tsc-multi"],
+				["attw", "@arethetypeswrong/cli"],
 			],
 		},
 		// These packages are independently versioned and released, but we use pnpm workspaces in single packages to work

diff --git a/packages/dds/merge-tree/.npmignore b/packages/dds/merge-tree/.npmignore
@@ -2,8 +2,12 @@ nyc
 *.log
 **/*.tsbuildinfo
 src/test
-dist/test
 **/_api-extractor-temp/**
 
 docs/
 .vscode/
+
+# We technically don't need to distribute the compiled test code, but because it's imported in the sequence package,
+# including it in the package makes it a lot easier to test using standardized tools. If we don't include the test code,
+# then the ./test export is invalid, so it will fail to import anywhere outside the repo.
+# dist/test
diff --git a/packages/dds/merge-tree/package.json b/packages/dds/merge-tree/package.json
@@ -11,8 +11,30 @@
 	"license": "MIT",
 	"author": "Microsoft and contributors",
 	"sideEffects": false,
-	"main": "dist/index.js",
-	"module": "lib/index.js",
+	"exports": {
+		".": {
+			"import": {
+				"types": "./lib/index.d.mts",
+				"default": "./lib/index.mjs"
+			},
+			"require": {
+				"types": "./dist/index.d.ts",
+				"default": "./dist/index.cjs"
+			}
+		},
+		"./dist/test": {
+			"import": {
+				"types": "./dist/test/index.d.ts",
+				"default": "./dist/test/index.cjs"
+			},
+			"require": {
+				"types": "./dist/test/index.d.ts",
+				"default": "./dist/test/index.cjs"
+			}
+		}
+	},
+	"main": "dist/index.cjs",
+	"module": "lib/index.mjs",
 	"types": "dist/index.d.ts",
 	"scripts": {
 		"api": "fluid-build . --task api",
@@ -22,8 +44,9 @@
 		"build:commonjs": "fluid-build . --task commonjs",
 		"build:compile": "fluid-build . --task compile",
 		"build:docs": "fluid-build . --task api",
-		"build:esnext": "tsc --project ./tsconfig.esnext.json",
-		"build:test": "tsc --project ./src/test/tsconfig.json",
+		"build:esnext": "tsc-multi --config ../../../common/build/build-common/tsc-multi.esm.json",
+		"build:rename-types": "renamer \"lib/**\" -f .d.ts -r .d.mts --force",
+		"build:test": "tsc-multi --config ./tsc-multi.test.json",
 		"check:are-the-types-wrong": "attw --pack",
 		"check:release-tags": "api-extractor run --local --config ./api-extractor-lint.json",
 		"ci:build:docs": "api-extractor run",
@@ -45,7 +68,7 @@
 		"test:mocha": "mocha --ignore \"dist/test/types/*\" --recursive dist/test",
 		"test:mocha:verbose": "cross-env FLUID_TEST_VERBOSE=1 npm run test:mocha",
 		"test:stress": "cross-env FUZZ_STRESS_RUN=1 FUZZ_TEST_COUNT=1 npm run test:mocha",
-		"tsc": "tsc",
+		"tsc": "tsc-multi --config ../../../common/build/build-common/tsc-multi.cjs.json",
 		"typetests:gen": "fluid-type-test-generator",
 		"typetests:prepare": "flub typetests --dir . --reset --previous --normalize"
 	},
@@ -107,7 +130,10 @@
 		"mocha-multi-reporters": "^1.5.1",
 		"moment": "^2.21.0",
 		"prettier": "~3.0.3",
+		"renamer": "^4.0.0",
+		"replace-in-file": "^6.3.5",
 		"rimraf": "^4.4.0",
+		"tsc-multi": "^1.1.0",
 		"typescript": "~5.1.6"
 	},
 	"fluidBuild": {

diff --git a/packages/dds/merge-tree/src/test/client.localReference.spec.ts b/packages/dds/merge-tree/src/test/client.localReference.spec.ts
@@ -15,7 +15,7 @@ import { setValidateRefCount, LocalReferencePosition, SlidingPreference } from "
 import { getSlideToSegoff } from "../mergeTree";
 import { createClientsAtInitialState } from "./testClientLogger";
 import { validateRefCount } from "./testUtils";
-import { TestClient } from "./";
+import { TestClient } from "./testClient";
 
 function getSlideOnRemoveReferencePosition(
 	client: Client,

diff --git a/packages/dds/merge-tree/src/test/index.ts b/packages/dds/merge-tree/src/test/index.ts
@@ -126,4 +126,4 @@ export {
 	TrackingGroupCollection,
 	UnassignedSequenceNumber,
 	UniversalSequenceNumber,
-} from "..";
+} from "../index";
diff --git a/packages/dds/merge-tree/src/test/snapshot.utils.ts b/packages/dds/merge-tree/src/test/snapshot.utils.ts
@@ -12,9 +12,11 @@ import { MockStorage } from "@fluidframework/test-runtime-utils";
 import { IMergeTreeOp, ReferenceType } from "../ops";
 import { SnapshotV1 } from "../snapshotV1";
 import { IMergeTreeOptions } from "../mergeTree";
+import { PropertySet } from "../properties";
+import { ISegment } from "../mergeTreeNodes";
 import { createClientsAtInitialState } from "./testClientLogger";
 import { TestSerializer } from "./testSerializer";
-import { ISegment, PropertySet, TestClient } from ".";
+import { TestClient } from "./testClient";
 
 // Reconstitutes a MergeTree client from a summary
 export async function loadSnapshot(summary: ISummaryTree, options?: IMergeTreeOptions) {

diff --git a/packages/dds/merge-tree/src/test/snapshotlegacy.spec.ts b/packages/dds/merge-tree/src/test/snapshotlegacy.spec.ts
@@ -15,7 +15,7 @@ import {
 } from "../attributionPolicy";
 import { TestSerializer } from "./testSerializer";
 import { createClientsAtInitialState } from "./testClientLogger";
-import { TestClient } from ".";
+import { TestClient } from "./testClient";
 
 describe("snapshot", () => {
 	it("header only", async () => {

diff --git a/packages/dds/merge-tree/tsc-multi.test.json b/packages/dds/merge-tree/tsc-multi.test.json
@@ -0,0 +1,10 @@
+{
+	"targets": [
+		{
+			"extname": ".cjs",
+			"module": "Node16",
+			"moduleResolution": "Node16"
+		}
+	],
+	"projects": ["./tsconfig.json", "./src/test/tsconfig.json"]
+}
diff --git a/packages/dds/merge-tree/tsconfig.esnext.json b/packages/dds/merge-tree/tsconfig.esnext.json
diff --git a/packages/dds/sequence/package.json b/packages/dds/sequence/package.json
@@ -11,8 +11,20 @@
 	"license": "MIT",
 	"author": "Microsoft and contributors",
 	"sideEffects": false,
-	"main": "dist/index.js",
-	"module": "lib/index.js",
+	"exports": {
+		".": {
+			"import": {
+				"types": "./lib/index.d.mts",
+				"default": "./lib/index.mjs"
+			},
+			"require": {
+				"types": "./dist/index.d.ts",
+				"default": "./dist/index.cjs"
+			}
+		}
+	},
+	"main": "dist/index.cjs",
+	"module": "lib/index.mjs",
 	"types": "dist/index.d.ts",
 	"scripts": {
 		"api": "fluid-build . --task api",
@@ -22,9 +34,10 @@
 		"build:commonjs": "fluid-build . --task commonjs",
 		"build:compile": "fluid-build . --task compile",
 		"build:docs": "fluid-build . --task api",
-		"build:esnext": "tsc --project ./tsconfig.esnext.json",
+		"build:esnext": "tsc-multi --config ../../../common/build/build-common/tsc-multi.esm.json",
 		"build:genver": "gen-version",
-		"build:test": "tsc --project ./src/test/tsconfig.json",
+		"build:rename-types": "renamer \"lib/**\" -f .d.ts -r .d.mts --force",
+		"build:test": "tsc-multi --config ./tsc-multi.test.json",
 		"check:are-the-types-wrong": "attw --pack",
 		"check:release-tags": "api-extractor run --local --config ./api-extractor-lint.json",
 		"ci:build:docs": "api-extractor run",
@@ -44,11 +57,11 @@
 		"test:coverage": "c8 npm test",
 		"test:memory": "mocha --config src/test/memory/.mocharc.js",
 		"test:memory-profiling:report": "mocha --config src/test/memory/.mocharc.js",
-		"test:mocha": "mocha --ignore \"dist/test/memory/**/*\" --recursive \"dist/test/**/*.spec.js\" -r node_modules/@fluidframework/mocha-test-setup",
+		"test:mocha": "mocha --ignore \"dist/test/memory/**/*\" --recursive \"dist/test/**/*.spec.?js\" -r node_modules/@fluidframework/mocha-test-setup",
 		"test:mocha:verbose": "cross-env FLUID_TEST_VERBOSE=1 npm run test:mocha",
 		"test:newsnapfiles": "node dist/test/createSnapshotFiles.js",
 		"test:stress": "cross-env FUZZ_TEST_COUNT=100 FUZZ_STRESS_RUN=true mocha --ignore \"dist/test/memory/**/*\" --recursive \"dist/test/**/*.fuzz.spec.js\" -r @fluidframework/mocha-test-setup",
-		"tsc": "tsc",
+		"tsc": "tsc-multi --config ../../../common/build/build-common/tsc-multi.cjs.json",
 		"typetests:gen": "fluid-type-test-generator",
 		"typetests:prepare": "flub typetests --dir . --reset --previous --normalize"
 	},
@@ -57,12 +70,12 @@
 		"cache-dir": "nyc/.cache",
 		"exclude": [
 			"src/test/**/*.ts",
-			"dist/test/**/*.js"
+			"dist/test/**/*.?js"
 		],
 		"exclude-after-remap": false,
 		"include": [
 			"src/**/*.ts",
-			"dist/**/*.js"
+			"dist/**/*.?js"
 		],
 		"report-dir": "nyc/report",
 		"reporter": [
@@ -113,7 +126,10 @@
 		"moment": "^2.21.0",
 		"prettier": "~3.0.3",
 		"random-js": "^2.1.0",
+		"renamer": "^4.0.0",
+		"replace-in-file": "^6.3.5",
 		"rimraf": "^4.4.0",
+		"tsc-multi": "^1.1.0",
 		"typescript": "~5.1.6"
 	},
 	"fluidBuild": {

diff --git a/packages/dds/sequence/src/test/partialLoad.spec.ts b/packages/dds/sequence/src/test/partialLoad.spec.ts
@@ -12,7 +12,7 @@ import {
 import { ReferenceType } from "@fluidframework/merge-tree";
 import { IChannelServices } from "@fluidframework/datastore-definitions";
 import { ISummaryTree } from "@fluidframework/protocol-definitions";
-import { SharedStringFactory, SharedString } from "..";
+import { SharedStringFactory, SharedString } from "../index";
 
 function applyOperations(
 	sharedString: SharedString,

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

diff --git a/packages/dds/sequence/src/test/subSequence.spec.ts b/packages/dds/sequence/src/test/subSequence.spec.ts
@@ -11,7 +11,7 @@ import {
 	PropertySet,
 } from "@fluidframework/merge-tree";
 // eslint-disable-next-line import/no-internal-modules
-import { TestClient } from "@fluidframework/merge-tree/dist/test/testClient";
+import { TestClient } from "@fluidframework/merge-tree/dist/test";
 import { SubSequence } from "../sharedSequence";
 
 const clientNames = ["Ed", "Ted", "Ned", "Harv", "Marv", "Glenda", "Susan"];

diff --git a/packages/dds/sequence/src/test/tsconfig.json b/packages/dds/sequence/src/test/tsconfig.json
@@ -13,7 +13,9 @@
 		"rootDir": "./",
 		"outDir": "../../dist/test",
 		"types": ["mocha"],
-		"noUnusedLocals": false, // Need it so memory tests can declare local variables just for the sake of keeping things in memory,
 		"noImplicitAny": false,
+		"noUnusedLocals": false, // Need it so memory tests can declare local variables just for the sake of keeping things in memory,
+		"module": "Node16",
+		"moduleResolution": "Node16",
 	},
 }
diff --git a/packages/dds/sequence/tsc-multi.test.json b/packages/dds/sequence/tsc-multi.test.json
@@ -0,0 +1,10 @@
+{
+	"targets": [
+		{
+			"extname": ".cjs",
+			"module": "Node16",
+			"moduleResolution": "Node16"
+		}
+	],
+	"projects": ["./tsconfig.json", "./src/test/tsconfig.json"]
+}
diff --git a/packages/dds/sequence/tsconfig.esnext.json b/packages/dds/sequence/tsconfig.esnext.json
diff --git a/packages/runtime/id-compressor/package.json b/packages/runtime/id-compressor/package.json
@@ -73,6 +73,7 @@
 		"uuid": "^9.0.0"
 	},
 	"devDependencies": {
+		"@arethetypeswrong/cli": "^0.13.3",
 		"@fluid-private/stochastic-test-utils": "workspace:~",
 		"@fluid-tools/benchmark": "^0.48.0",
 		"@fluid-tools/build-cli": "^0.28.0",