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.

Sign up for GitHub

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

build(merge-tree,sequence): Fix type generation for ESM and add exports field #18825

Merged
merged 6 commits into from
Dec 14, 2023
Merged

build(merge-tree,sequence): Fix type generation for ESM and add exports field #18825

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e343e46
baseline config changes for all node16 PRs
tylerbutler Dec 14, 2023
bd79b40
merge tree and sequence
tylerbutler Dec 14, 2023
56cc9c6
lockfile
tylerbutler Dec 14, 2023
130a6dd
policy
tylerbutler Dec 14, 2023
3357baa
Merge branch 'main' into node16-part-2
tylerbutler Dec 14, 2023
f9bbe73
Apply suggestions from code review
tylerbutler Dec 14, 2023
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
2 changes: 1 addition & 1 deletion common/build/build-common/tsc-multi.test.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"]
}
34 changes: 27 additions & 7 deletions fluidBuild.config.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -23,7 +23,14 @@ module.exports = {
script: false,
},
"compile": {
dependsOn: ["commonjs", "build:esnext", "build:test", "build:copy"],
dependsOn: [
"commonjs",
"build:esnext",
"build:test",
"build:copy",
// "^build:rename-types",
Josmithr marked this conversation as resolved.
Show resolved Hide resolved
tylerbutler marked this conversation as resolved.
Show resolved Hide resolved
"build:rename-types",
],
script: false,
},
"commonjs": {
Expand Down Expand Up @@ -58,10 +65,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,
Expand Down Expand Up @@ -159,9 +167,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": [
Expand Down Expand Up @@ -217,9 +233,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": [
Expand Down Expand Up @@ -315,6 +332,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
Expand Down
6 changes: 5 additions & 1 deletion packages/dds/merge-tree/.npmignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
36 changes: 31 additions & 5 deletions packages/dds/merge-tree/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to just call this ./test? Would clean up some import patterns nicely.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but in a future change, because doing that require more churn.

"import": {
"types": "./dist/test/index.d.ts",
"default": "./dist/test/index.cjs"
},
"require": {
"types": "./dist/test/index.d.ts",
"default": "./dist/test/index.cjs"
}
}
Comment on lines +25 to +34
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"./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"
}
}
"./dist/test": {
"types": "./dist/test/index.d.ts",
"default": "./dist/test/index.cjs"
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not going to risk changing it, though.

},
"main": "dist/index.cjs",
"module": "lib/index.mjs",
"types": "dist/index.d.ts",
"scripts": {
"api": "fluid-build . --task api",
Expand All @@ -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",
Expand All @@ -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"
},
Expand Down Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion packages/dds/merge-tree/src/test/client.localReference.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion packages/dds/merge-tree/src/test/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,4 @@ export {
TrackingGroupCollection,
UnassignedSequenceNumber,
UniversalSequenceNumber,
} from "..";
} from "../index";
Josmithr marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 3 additions & 1 deletion packages/dds/merge-tree/src/test/snapshot.utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion packages/dds/merge-tree/src/test/snapshotlegacy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
10 changes: 10 additions & 0 deletions packages/dds/merge-tree/tsc-multi.test.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"targets": [
{
"extname": ".cjs",
"module": "Node16",
"moduleResolution": "Node16"
}
],
"projects": ["./tsconfig.json", "./src/test/tsconfig.json"]
}
6 changes: 0 additions & 6 deletions packages/dds/merge-tree/tsconfig.esnext.json
View file
Open in desktop

This file was deleted.

32 changes: 24 additions & 8 deletions packages/dds/sequence/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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"
},
Expand All @@ -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": [
Expand Down Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion packages/dds/sequence/src/test/partialLoad.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion packages/dds/sequence/src/test/sequenceDeltaEvent.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion packages/dds/sequence/src/test/subSequence.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"];
Expand Down
4 changes: 3 additions & 1 deletion packages/dds/sequence/src/test/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
}
10 changes: 10 additions & 0 deletions packages/dds/sequence/tsc-multi.test.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"targets": [
{
"extname": ".cjs",
"module": "Node16",
"moduleResolution": "Node16"
}
],
"projects": ["./tsconfig.json", "./src/test/tsconfig.json"]
}
6 changes: 0 additions & 6 deletions packages/dds/sequence/tsconfig.esnext.json
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions packages/runtime/id-compressor/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading