build(tree): Make compatible with moduleResolution: node16 #18882

tylerbutler · 2023-12-16T22:31:55Z

Updates the tree package to build ESM using tsc-multi, create valid ESM types, and adds an exports field and testing using arethetypeswrong.

This change also required more changes to tsc-multi to handle the types tree uses. Those changes are included here by updating the patch.

alexvy86 · 2023-12-22T21:28:06Z

packages/dds/tree/src/domains/schemaBuilder.ts

@@ -273,27 +273,27 @@ export class SchemaBuilder<
 	/**
 	 * {@link leaf.number}
 	 */
-	public readonly number = leaf.number;
+	public readonly number: typeof leaf.number = leaf.number;


Some explicit types added where they were simple, to reduce the number of dynamic imports in d.?ts files.

We shouldn't have to make any source changes to get rollups and generate API reports.
Alternate solution for tree #19141

alexvy86 · 2023-12-22T21:28:29Z

packages/dds/tree/src/feature-libraries/default-schema/defaultFieldKinds.ts

-		allowsTreeSchemaIdentifierSuperset(types, other.types),
-	new Set([]),
-);
+export const optional: FieldKindWithEditor<OptionalFieldEditor, Multiplicity.Optional, "Optional"> =


Some explicit types added where they were simple, to reduce the number of dynamic imports in d.?ts files (same below in this file).

alexvy86 · 2023-12-22T21:28:38Z

packages/dds/tree/src/feature-libraries/typed-schema/typedTreeSchema.ts

@@ -328,7 +328,10 @@ export class TreeFieldSchema<
 	/**
 	 * Schema for a field which must always be empty.
 	 */
-	public static readonly empty = TreeFieldSchema.create(FieldKinds.forbidden, []);
+	public static readonly empty: TreeFieldSchema<Forbidden, readonly []> = TreeFieldSchema.create(


Some explicit types added where they were simple, to reduce the number of dynamic imports in d.?ts files.

alexvy86 · 2023-12-22T21:29:36Z

packages/dds/tree/src/feature-libraries/optional-field/index.ts

@@ -8,6 +8,7 @@ export { OptionalChangeset, RegisterId } from "./optionalFieldChangeTypes";
 export {
 	optionalChangeHandler,
 	optionalFieldEditor,
+	OptionalFieldEditor,


This was done so I could add explicit types to some things, following what sequence field already did in terms of exports.

alexvy86 · 2023-12-22T21:30:18Z

packages/dds/tree/src/core/schema-stored/storedSchemaRepository.ts

@@ -54,13 +54,17 @@ export interface MutableTreeStoredSchema extends TreeStoredSchemaSubscription {
 	apply(newSchema: TreeStoredSchema): void;
 }

+type eventsEmitterType = ISubscribable<SchemaEvents> &


Some explicit types added where they were simple, to reduce the number of dynamic imports in d.?ts files.

msfluid-bot · 2023-12-22T21:49:03Z

⯅ @fluid-example/bundle-size-tests: +209 Bytes

Metric Name	Baseline Size	Compare Size	Size Diff
aqueduct.js	739.44 KB	739.48 KB	⯅ +33 Bytes
connectionState.js	680 Bytes	680 Bytes	■ No change
containerRuntime.js	538.32 KB	538.33 KB	⯅ +11 Bytes
loader.js	166.08 KB	166.1 KB	⯅ +22 Bytes
map.js	556.87 KB	556.89 KB	⯅ +22 Bytes
matrix.js	655.14 KB	655.17 KB	⯅ +22 Bytes
odspDriver.js	89.31 KB	89.35 KB	⯅ +33 Bytes
odspPrefetchSnapshot.js	41.58 KB	41.61 KB	⯅ +22 Bytes
sharedString.js	673.91 KB	673.93 KB	⯅ +22 Bytes
sharedTree.js	787.83 KB	787.84 KB	⯅ +11 Bytes
Total Size	4.27 MB	4.27 MB	⯅ +209 Bytes

Baseline commit: 72b15bf

Generated by 🚫 dangerJS against 325bf6f

experimental/dds/tree/package.json

experimental/framework/tree-react-api/tsc-multi.test.json

jason-ha · 2024-01-06T20:41:34Z

experimental/dds/tree/package.json

 	"main": "dist/index.js",
-	"module": "lib/index.js",
+	"module": "lib/index.mjs",


Should just be removed - module is not supported and never will be

jason-ha · 2024-01-06T20:45:26Z

packages/dds/tree/package.json

@@ -12,17 +12,19 @@
 	"author": "Microsoft and contributors",
 	"sideEffects": false,
 	"main": "dist/index.js",
-	"module": "lib/index.js",
+	"module": "lib/index.mjs",


Should just be removed - module is not supported

jason-ha · 2024-01-06T20:46:32Z

packages/dds/tree/package.json

 		"bench": "mocha --timeout 999999 --perfMode --parentProcess --fgrep @Benchmark --reporter @fluid-tools/benchmark/dist/MochaReporter.js",
 		"bench:profile": "mocha --v8-prof --v8-logfile=profile.log --v8-no-logfile-per-isolate --timeout 999999 --perfMode --fgrep @Benchmark --reporter @fluid-tools/benchmark/dist/MochaReporter.js && node --prof-process profile.log > profile.txt && rimraf profile.log && echo See results in profile.txt",
 		"build": "fluid-build . --task build",
 		"build:commonjs": "fluid-build . --task commonjs",
 		"build:compile": "fluid-build . --task compile",
 		"build:docs": "api-extractor run --local",
-		"build:esnext": "tsc --project ./tsconfig.esnext.json",
-		"build:genver": "gen-version",
+		"build:esnext": "tsc-multi --config ../../../common/build/build-common/tsc-multi.esm.json",


I think we should prefer ESM first general but at least for tree. Make sure we see all of the issues with ESM without filtering.

jason-ha · 2024-01-06T20:50:47Z

packages/dds/tree/package.json

@@ -12,17 +12,19 @@
 	"author": "Microsoft and contributors",
 	"sideEffects": false,
 	"main": "dist/index.js",


"type" should be set (prefer "module" with esm as first class.)
and "exports" should be added.

Agree on removing the main/module field ultimately, but I'd like to do it separately, and ideally for all packages in the release group. Also, are you sure that removing these is not a breaking change? I thought at least webpack used the module field, but haven't yet confirmed whether removing it breaks our own examples. That's the other (minor) reason I want to do it all at once - so I can make only one "breaking change" note.

jason-ha · 2024-01-08T10:52:48Z

packages/dds/tree/api-report/tree.api.md

@@ -4,6 +4,8 @@

 ```ts

+import { FieldKind as FieldKind_2 } from './schemaTypes.js';
+import { FieldSchema as FieldSchema_2 } from './schemaTypes.js';
 import { FluidObject } from '@fluidframework/core-interfaces';


These are not valid which means the API extractor generated .d.ts files are probably incorrect.

Here is an example log when I had such gunk: https://dev.azure.com/fluidframework/public/_build/results?buildId=228628&view=logs&j=3dc8fd7e-4368-5a92-293e-d53cefc8c4b3&t=bfe27d5f-9219-57c0-0d67-1b2946a32921

It's worrisome to me that in the example you linked, the tools involved were only tsc and api-extractor. Does that imply that api-extractor itself doesn't like the Tree types? Maybe we should try updating api-extractor? Looks like the recent minor release added support for TypeScript 5.3 - https://github.com/microsoft/rushstack/blob/main/apps/api-extractor/CHANGELOG.md#7390

@Josmithr FYI.

I was guessing it isn't a Tree type thing but something with moduleResolution support.

I did a test with updated api-extractor for my branch and results were not great.
First I tried exactly as I had things and then with tree typescript set to 5.3.3 which the changelog said is supported:

@fluidframework/tree: error during command 'api-extractor run --local --config ./api-extractor-lint.json' (exit code 1) @fluidframework/tree: api-extractor 7.39.1 - https://api-extractor.com/ @fluidframework/tree: @fluidframework/tree: @fluidframework/tree: @fluidframework/tree: ERROR: Error parsing /home/jasonha/ff/FluidFramework/packages/dds/tree/api-extractor-lint.json: @fluidframework/tree: The "bundledPackages" list contains an invalid package name: "@fluidframework/.*" [ 8/12] x @fluidframework/tree: api-extractor run --local --config ./api-extractor-lint.json - 0.327s @fluidframework/tree: error during command 'api-extractor run --config ./api-extractor-cjs.json' (exit code 1) @fluidframework/tree: api-extractor 7.39.1 - https://api-extractor.com/ @fluidframework/tree: @fluidframework/tree: Analysis will use the bundled TypeScript version 5.1.6 @fluidframework/tree: *** The target project appears to use TypeScript 5.3.3 which is newer than the bundled compiler engine; consider upgrading API Extractor. @fluidframework/tree: @fluidframework/tree: @fluidframework/tree: ERROR: program.getResolvedModule is not a function [ 9/12] x @fluidframework/tree: api-extractor run --config ./api-extractor-cjs.json - 2.742s @fluidframework/tree: error during command 'api-extractor run --local --config ./api-extractor-esm.json' (exit code 1) @fluidframework/tree: api-extractor 7.39.1 - https://api-extractor.com/ @fluidframework/tree: @fluidframework/tree: Analysis will use the bundled TypeScript version 5.1.6 @fluidframework/tree: *** The target project appears to use TypeScript 5.3.3 which is newer than the bundled compiler engine; consider upgrading API Extractor. @fluidframework/tree: @fluidframework/tree: @fluidframework/tree: ERROR: program.getResolvedModule is not a function [10/12] x @fluidframework/tree: api-extractor run --local --config ./api-extractor-esm.json - 2.726s

These errors might be related to the manual api-extractor patch that @Josmithr has been managing. Presumably that patch will need to be regenerated.

Good call out. The patch is listed as for a specific version but might not be. I then also found that we were forcing 5.1.6 into api-extractor per pnpm override. When I removed those two things the main runs worked. The lint case still complains about malformed bundledPackages.
No change on the results for the different module resolutions and bad results.
I feel like I've seen such things in other packages, but maybe it was always when playing with module resolution. I think there was some checked in at some point and then something corrected it later.

Ha, ha. The patch is specifically about making bundledPackages support regex.

update tree and friends

c2657e7

build

3945131

github-actions bot added the public api change Changes to a public API label Dec 17, 2023

tylerbutler and others added 10 commits December 19, 2023 16:43

Merge branch 'main' into node16-tree

7fd1e68

add api-extractor

9c45c01

Merge branch 'main' into node16-tree

321350b

Add simple explicit types

629a5fb

Adding some explicit types

5a73c61

Update patch

8ae0f78

Merge branch 'main' into node16-tree

b80f612

Restore stuff deleted incorrectly

d3fa7f6

Merge branch 'main' into node16-tree

75f7a95

Updated api report files

165a07d

github-actions bot added area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct and removed area: runtime Runtime related issues labels Dec 22, 2023

alexvy86 reviewed Dec 22, 2023

View reviewed changes

Remove renamer

ccaa26a

github-actions bot removed the area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct label Dec 22, 2023

Squished branch: node16-tree

538353c

tylerbutler force-pushed the node16-tree branch from 6a16a5d to 538353c Compare January 4, 2024 23:41

tylerbutler added 5 commits January 4, 2024 16:32

Merge commit '6a16a5d5d3af3cf82e42df986a455b08a68798fe' into node16-tree

a1b91aa

fixes

f189eb7

Merge branch 'main' into node16-tree

058bdfb

Merge branch 'main' into node16-tree

960d61b

fix entrypoint

acf44d8

tylerbutler marked this pull request as ready for review January 5, 2024 22:18

tylerbutler requested review from msfluid-bot and a team as code owners January 5, 2024 22:18

Merge branch 'main' into node16-tree

3bdad72

tylerbutler commented Jan 5, 2024

View reviewed changes

experimental/dds/tree/package.json Outdated Show resolved Hide resolved

tylerbutler commented Jan 5, 2024

View reviewed changes

experimental/dds/tree/package.json Outdated Show resolved Hide resolved

tylerbutler commented Jan 5, 2024

View reviewed changes

experimental/dds/tree/package.json Outdated Show resolved Hide resolved

tylerbutler commented Jan 5, 2024

View reviewed changes

experimental/framework/tree-react-api/tsc-multi.test.json Outdated Show resolved Hide resolved

use tsc

e10a943

github-actions bot removed the area: dev experience Improving the experience of devs building on top of fluid label Jan 5, 2024

Merge branch 'main' into node16-tree

325bf6f

jason-ha reviewed Jan 6, 2024

View reviewed changes

jason-ha reviewed Jan 8, 2024

View reviewed changes

tylerbutler closed this Feb 7, 2024

tylerbutler deleted the node16-tree branch March 19, 2024 20:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build(tree): Make compatible with moduleResolution: node16 #18882

build(tree): Make compatible with moduleResolution: node16 #18882

tylerbutler commented Dec 16, 2023 •

edited

Loading

alexvy86 Dec 22, 2023

jason-ha Jan 8, 2024

alexvy86 Dec 22, 2023

alexvy86 Dec 22, 2023

alexvy86 Dec 22, 2023

alexvy86 Dec 22, 2023

msfluid-bot commented Dec 22, 2023 •

edited

Loading

jason-ha Jan 6, 2024

jason-ha Jan 6, 2024

jason-ha Jan 6, 2024

jason-ha Jan 6, 2024

tylerbutler Jan 6, 2024

jason-ha Jan 8, 2024

jason-ha Jan 8, 2024

tylerbutler Jan 8, 2024

jason-ha Jan 8, 2024

tylerbutler Jan 8, 2024

jason-ha Jan 8, 2024

jason-ha Jan 8, 2024

build(tree): Make compatible with moduleResolution: node16 #18882

build(tree): Make compatible with moduleResolution: node16 #18882

Conversation

tylerbutler commented Dec 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msfluid-bot commented Dec 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tylerbutler commented Dec 16, 2023 •

edited

Loading

msfluid-bot commented Dec 22, 2023 •

edited

Loading