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

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

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
9 changes: 5 additions & 4 deletions examples/apps/tree-comparison/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
{
"extends": "@fluidframework/build-common/ts-common-config.json",
"extends": [
"../../../common/build/build-common/tsconfig.base.json",
"../../../common/build/build-common/tsconfig.esm-only.json",
],
"include": ["src/**/*", "tests/**/*"],
"compilerOptions": {
"module": "esnext",
"outDir": "lib",
"jsx": "react",
"types": ["jest", "puppeteer", "jest-environment-puppeteer", "expect-puppeteer", "react"],
},
"include": ["src/**/*", "tests/**/*"],
}
35 changes: 30 additions & 5 deletions experimental/dds/tree/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,37 @@
"license": "MIT",
"author": "Microsoft and contributors",
"sideEffects": false,
"exports": {
".": {
"import": {
"types": "./lib/index.d.mts",
"default": "./lib/index.mjs"
},
"require": {
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
}
},
"./test/EditLog": {
"import": {
"types": "./lib/EditLog.d.mts",
"default": "./lib/EditLog.mjs"
},
"require": {
"types": "./dist/EditLog.d.ts",
"default": "./dist/EditLog.js"
}
}
},
"main": "dist/index.js",
"module": "lib/index.js",
"module": "lib/index.mjs",
Copy link
Contributor

Choose a reason for hiding this comment

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

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

"types": "dist/index.d.ts",
"scripts": {
"build": "fluid-build . --task build",
"build:compile": "fluid-build . --task compile",
"build:docs": "api-extractor run --local",
"build:esnext": "tsc --project ./tsconfig.esnext.json",
"build:esnext": "tsc-multi --config ../../../common/build/build-common/tsc-multi.esm.json",
"check:are-the-types-wrong": "attw --pack . --entrypoints .",
"check:release-tags": "api-extractor run --local --config ./api-extractor-lint.json",
"ci:build:docs": "api-extractor run",
"clean": "rimraf --glob dist lib \"**/*.tsbuildinfo\" \"**/*.build.log\" _api-extractor-temp nyc",
Expand All @@ -28,14 +51,14 @@
"lint": "npm run prettier && npm run check:release-tags && npm run eslint",
"lint:fix": "npm run prettier:fix && npm run eslint:fix",
"postpack": "tar -cf ./experimental-tree.test-files.tar ./src/test ./dist/test",
"perf": "cross-env FLUID_TEST_VERBOSE=1 mocha \"dist/**/*.tests.js\" --node-option unhandled-rejections=strict,expose-gc --exit -r node_modules/@fluidframework/mocha-test-setup --perfMode --fgrep @Benchmark --reporter @fluid-tools/benchmark/dist/MochaReporter.js --timeout 30000",
"perf": "cross-env FLUID_TEST_VERBOSE=1 mocha \"dist/**/*.tests.*js\" --node-option unhandled-rejections=strict,expose-gc --exit -r node_modules/@fluidframework/mocha-test-setup --perfMode --fgrep @Benchmark --reporter @fluid-tools/benchmark/dist/MochaReporter.js --timeout 30000",
"perf:measure": "npm run perf -- --fgrep @Measurement",
"prettier": "prettier --check . --cache --ignore-path ../../../.prettierignore",
"prettier:fix": "prettier --write . --cache --ignore-path ../../../.prettierignore",
"test": "npm run test:mocha",
"test:benchmark:report": "mocha \"dist/**/*.tests.js\" --node-option unhandled-rejections=strict,expose-gc --exit --perfMode --fgrep @Benchmark -r node_modules/@fluidframework/mocha-test-setup --reporter @fluid-tools/benchmark/dist/MochaReporter.js --timeout 60000",
"test:benchmark:report": "mocha \"dist/**/*.tests.*js\" --node-option unhandled-rejections=strict,expose-gc --exit --perfMode --fgrep @Benchmark -r node_modules/@fluidframework/mocha-test-setup --reporter @fluid-tools/benchmark/dist/MochaReporter.js --timeout 60000",
"test:coverage": "c8 npm test",
"test:mocha": "mocha \"dist/**/*.tests.js\" --exit -r node_modules/@fluidframework/mocha-test-setup",
"test:mocha": "mocha \"dist/**/*.tests.*js\" --exit -r node_modules/@fluidframework/mocha-test-setup",
"test:mocha:verbose": "cross-env FLUID_TEST_VERBOSE=1 npm run test:mocha",
"test:stress": "cross-env FUZZ_TEST_COUNT=10 FUZZ_STRESS_RUN=true mocha \"dist/**/*.fuzz.tests.js\" --exit -r @fluidframework/mocha-test-setup",
"tsc": "tsc"
Expand All @@ -59,6 +82,7 @@
"uuid": "^9.0.0"
},
"devDependencies": {
"@arethetypeswrong/cli": "^0.13.3",
"@fluid-private/stochastic-test-utils": "workspace:~",
"@fluid-private/test-drivers": "workspace:~",
"@fluid-tools/benchmark": "^0.48.0",
Expand Down Expand Up @@ -91,6 +115,7 @@
"moment": "^2.21.0",
"prettier": "~3.0.3",
"rimraf": "^4.4.0",
"tsc-multi": "^1.1.0",
"typescript": "~5.1.6"
},
"typeValidation": {
Expand Down
7 changes: 0 additions & 7 deletions experimental/dds/tree/tsconfig.esnext.json

This file was deleted.

14 changes: 8 additions & 6 deletions experimental/dds/tree/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
{
"extends": "@fluidframework/build-common/ts-common-config.json",
"extends": [
"../../../common/build/build-common/tsconfig.base.json",
"../../../common/build/build-common/tsconfig.cjs.json"
],
"include": ["src/**/*"],
"exclude": ["dist", "node_modules"],
"compilerOptions": {
"composite": true,
"rootDir": "./src",
"outDir": "./dist",
"downlevelIteration": true,
"noUnusedLocals": false,
"outDir": "./dist",
"rootDir": "./src",
"types": ["mocha"]
},
"include": ["src/**/*"]
}
}
4 changes: 4 additions & 0 deletions packages/dds/tree/api-extractor-esm.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json",
"extends": "../../../common/build/build-common/api-extractor-base-esm.json"
}
38 changes: 24 additions & 14 deletions packages/dds/tree/api-report/tree.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

import { IChannel } from '@fluidframework/datastore-definitions';
import { IChannelAttributes } from '@fluidframework/datastore-definitions';
Expand All @@ -14,13 +16,21 @@ import { IFluidDataStoreRuntime } from '@fluidframework/datastore-definitions';
import { IFluidHandle } from '@fluidframework/core-interfaces';
import { IFluidLoadable } from '@fluidframework/core-interfaces';
import { IGarbageCollectionData } from '@fluidframework/runtime-definitions';
import { InsertableObjectFromSchemaRecord as InsertableObjectFromSchemaRecord_2 } from './schemaTypes.js';
import { ISharedObject } from '@fluidframework/shared-object-base';
import { ISummaryTreeWithStats } from '@fluidframework/runtime-definitions';
import { ITelemetryContext } from '@fluidframework/runtime-definitions';
import { LeafNodeSchema as LeafNodeSchema_2 } from '../feature-libraries/index.js';
import { ObjectFromSchemaRecord as ObjectFromSchemaRecord_2 } from './schemaTypes.js';
import { SchemaLibrary as SchemaLibrary_2 } from '../feature-libraries/schemaBuilderBase.js';
import { SessionSpaceCompressedId } from '@fluidframework/id-compressor';
import { StableId } from '@fluidframework/id-compressor';
import type { Static } from '@sinclair/typebox';
import { TreeNode as TreeNode_2 } from './types.js';
import { TreeNodeSchema as TreeNodeSchema_2 } from './schemaTypes.js';
import { TreeNodeSchemaClass as TreeNodeSchemaClass_2 } from './schemaTypes.js';
import type { TSchema } from '@sinclair/typebox';
import { WithType as WithType_2 } from './schemaTypes.js';

// @internal
export function adaptEnum<TScope extends string, const TEnum extends Record<string, string>>(factory: SchemaFactory<TScope>, members: TEnum): (<TValue extends TEnum[keyof TEnum]>(value: TValue) => object & TreeNode & ObjectFromSchemaRecord<EmptyObject> & {
Expand Down Expand Up @@ -1091,14 +1101,14 @@ export type LazyTreeNodeSchema = FlexTreeNodeSchema | (() => FlexTreeNodeSchema)

// @internal
export const leaf: {
number: LeafNodeSchema<"com.fluidframework.leaf.number", ValueSchema.Number>;
boolean: LeafNodeSchema<"com.fluidframework.leaf.boolean", ValueSchema.Boolean>;
string: LeafNodeSchema<"com.fluidframework.leaf.string", ValueSchema.String>;
handle: LeafNodeSchema<"com.fluidframework.leaf.handle", ValueSchema.FluidHandle>;
null: LeafNodeSchema<"com.fluidframework.leaf.null", ValueSchema.Null>;
primitives: readonly [LeafNodeSchema<"com.fluidframework.leaf.number", ValueSchema.Number>, LeafNodeSchema<"com.fluidframework.leaf.boolean", ValueSchema.Boolean>, LeafNodeSchema<"com.fluidframework.leaf.string", ValueSchema.String>];
all: readonly [LeafNodeSchema<"com.fluidframework.leaf.handle", ValueSchema.FluidHandle>, LeafNodeSchema<"com.fluidframework.leaf.null", ValueSchema.Null>, LeafNodeSchema<"com.fluidframework.leaf.number", ValueSchema.Number>, LeafNodeSchema<"com.fluidframework.leaf.boolean", ValueSchema.Boolean>, LeafNodeSchema<"com.fluidframework.leaf.string", ValueSchema.String>];
library: SchemaLibrary;
number: LeafNodeSchema_2<"com.fluidframework.leaf.number", ValueSchema.Number>;
boolean: LeafNodeSchema_2<"com.fluidframework.leaf.boolean", ValueSchema.Boolean>;
string: LeafNodeSchema_2<"com.fluidframework.leaf.string", ValueSchema.String>;
handle: LeafNodeSchema_2<"com.fluidframework.leaf.handle", ValueSchema.FluidHandle>;
null: LeafNodeSchema_2<"com.fluidframework.leaf.null", ValueSchema.Null>;
primitives: readonly [LeafNodeSchema_2<"com.fluidframework.leaf.number", ValueSchema.Number>, LeafNodeSchema_2<"com.fluidframework.leaf.boolean", ValueSchema.Boolean>, LeafNodeSchema_2<"com.fluidframework.leaf.string", ValueSchema.String>];
all: readonly [LeafNodeSchema_2<"com.fluidframework.leaf.handle", ValueSchema.FluidHandle>, LeafNodeSchema_2<"com.fluidframework.leaf.null", ValueSchema.Null>, LeafNodeSchema_2<"com.fluidframework.leaf.number", ValueSchema.Number>, LeafNodeSchema_2<"com.fluidframework.leaf.boolean", ValueSchema.Boolean>, LeafNodeSchema_2<"com.fluidframework.leaf.string", ValueSchema.String>];
library: SchemaLibrary_2;
};

// @internal (undocumented)
Expand Down Expand Up @@ -1603,12 +1613,12 @@ export class test_RecursiveObject extends test_RecursiveObject_base {
}

// @internal
export const test_RecursiveObject_base: TreeNodeSchemaClass<"Test Recursive Domain.testObject", NodeKind.Object, object & TreeNode & ObjectFromSchemaRecord< {
readonly recursive: FieldSchema<import("./schemaTypes.js").FieldKind.Optional, readonly [() => typeof test_RecursiveObject]>;
readonly number: TreeNodeSchema<"com.fluidframework.leaf.number", NodeKind.Leaf, number, number>;
}> & WithType<"Test Recursive Domain.testObject">, object & InsertableObjectFromSchemaRecord< {
readonly recursive: FieldSchema<import("./schemaTypes.js").FieldKind.Optional, readonly [() => typeof test_RecursiveObject]>;
readonly number: TreeNodeSchema<"com.fluidframework.leaf.number", NodeKind.Leaf, number, number>;
export const test_RecursiveObject_base: TreeNodeSchemaClass_2<"Test Recursive Domain.testObject", NodeKind.Object, object & TreeNode_2 & ObjectFromSchemaRecord_2< {
readonly recursive: FieldSchema_2<FieldKind_2.Optional, readonly [() => typeof test_RecursiveObject]>;
readonly number: TreeNodeSchema_2<"com.fluidframework.leaf.number", NodeKind.Leaf, number, number>;
}> & WithType_2<"Test Recursive Domain.testObject">, object & InsertableObjectFromSchemaRecord_2< {
readonly recursive: FieldSchema_2<FieldKind_2.Optional, readonly [() => typeof test_RecursiveObject]>;
readonly number: TreeNodeSchema_2<"com.fluidframework.leaf.number", NodeKind.Leaf, number, number>;
}>, true>;

// @internal
Expand Down
21 changes: 18 additions & 3 deletions packages/dds/tree/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,19 @@
"author": "Microsoft and contributors",
"sideEffects": false,
"main": "dist/index.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

"module": "lib/index.js",
"module": "lib/index.mjs",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should just be removed - module is not supported

"types": "dist/index.d.ts",
"scripts": {
"api": "fluid-build . --task api",
"api-extractor:commonjs": "api-extractor run --local",
"api-extractor:esnext": "api-extractor run --config ./api-extractor-esm.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",
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

"build:test": "tsc --project ./src/test/tsconfig.json",
"check:are-the-types-wrong": "attw --pack . --entrypoints .",
"check:release-tags": "api-extractor run --local --config ./api-extractor-lint.json",
Expand Down Expand Up @@ -125,8 +127,21 @@
"prettier": "~3.0.3",
"rimraf": "^4.4.0",
"ts2esm": "^1.1.0",
"tsc-multi": "^1.1.0",
"typescript": "~5.1.6"
},
"fluidBuild": {
"tasks": {
"build:docs": {
"dependsOn": [
"...",
"api-extractor:commonjs",
"api-extractor:esnext"
],
"script": false
}
}
},
"typeValidation": {
"disabled": true,
"broken": {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { BTree } from "@tylerbu/sorted-btree-es6";
import { createEmitter, ISubscribable } from "../../events/index.js";
import { createEmitter, HasListeners, IEmitter, ISubscribable } from "../../events/index.js";
import { compareStrings } from "../../util/index.js";
import {
StoredSchemaCollection,
Expand Down Expand Up @@ -54,13 +54,17 @@ export interface MutableTreeStoredSchema extends TreeStoredSchemaSubscription {
apply(newSchema: TreeStoredSchema): void;
}

type eventsEmitterType = ISubscribable<SchemaEvents> &
Copy link
Contributor

Choose a reason for hiding this comment

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

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

IEmitter<SchemaEvents> &
HasListeners<SchemaEvents>;

/**
* Mutable TreeStoredSchema repository.
*/
export class TreeStoredSchemaRepository implements MutableTreeStoredSchema {
protected nodeSchemaData: BTree<TreeNodeSchemaIdentifier, TreeNodeStoredSchema>;
protected rootFieldSchemaData: TreeFieldStoredSchema;
protected readonly events = createEmitter<SchemaEvents>();
protected readonly events: eventsEmitterType = createEmitter<SchemaEvents>();

/**
* Copies in the provided schema. If `data` is an TreeStoredSchemaRepository, it will be cheap-cloned.
Expand Down
10 changes: 5 additions & 5 deletions packages/dds/tree/src/domains/schemaBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,27 +277,27 @@ export class SchemaBuilder<
/**
* {@link leaf.number}
*/
public readonly number = leaf.number;
public readonly number: typeof leaf.number = leaf.number;
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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


/**
* {@link leaf.boolean}
*/
public readonly boolean = leaf.boolean;
public readonly boolean: typeof leaf.boolean = leaf.boolean;

/**
* {@link leaf.string}
*/
public readonly string = leaf.string;
public readonly string: typeof leaf.string = leaf.string;

/**
* {@link leaf.handle}
*/
public readonly handle = leaf.handle;
public readonly handle: typeof leaf.handle = leaf.handle;

/**
* {@link leaf.null}
*/
public readonly null = leaf.null;
public readonly null: typeof leaf.null = leaf.null;

/**
* Function which can be used for its compile time side-effects to tweak the evaluation order of recursive types to make them compile.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@ import {
referenceFreeFieldChangeRebaser,
FieldKindWithEditor,
} from "../modular-schema/index.js";
import { sequenceFieldChangeHandler } from "../sequence-field/index.js";
import { SequenceFieldEditor, sequenceFieldChangeHandler } from "../sequence-field/index.js";
import {
noChangeCodecFamily,
OptionalChangeset,
optionalChangeHandler,
optionalFieldEditor,
OptionalFieldEditor,
} from "../optional-field/index.js";
import { Multiplicity } from "../multiplicity.js";

Expand Down Expand Up @@ -59,16 +60,17 @@ const optionalIdentifier = "Optional";
/**
* 0 or 1 items.
*/
export const optional = new FieldKindWithEditor(
optionalIdentifier,
Multiplicity.Optional,
optionalChangeHandler,
(types, other) =>
(other.kind.identifier === sequence.identifier ||
other.kind.identifier === optionalIdentifier) &&
allowsTreeSchemaIdentifierSuperset(types, other.types),
new Set([]),
);
export const optional: FieldKindWithEditor<OptionalFieldEditor, Multiplicity.Optional, "Optional"> =
Copy link
Contributor

Choose a reason for hiding this comment

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

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

new FieldKindWithEditor(
optionalIdentifier,
Multiplicity.Optional,
optionalChangeHandler,
(types, other) =>
(other.kind.identifier === sequence.identifier ||
other.kind.identifier === optionalIdentifier) &&
allowsTreeSchemaIdentifierSuperset(types, other.types),
new Set([]),
);

export const valueFieldEditor: ValueFieldEditor = {
...optionalFieldEditor,
Expand Down Expand Up @@ -104,16 +106,17 @@ const sequenceIdentifier = "Sequence";
/**
* 0 or more items.
*/
export const sequence = new FieldKindWithEditor(
sequenceIdentifier,
Multiplicity.Sequence,
sequenceFieldChangeHandler,
(types, other) =>
other.kind.identifier === sequenceIdentifier &&
allowsTreeSchemaIdentifierSuperset(types, other.types),
// TODO: add normalizer/importers for handling ops from other kinds.
new Set([]),
);
export const sequence: FieldKindWithEditor<SequenceFieldEditor, Multiplicity.Sequence, "Sequence"> =
new FieldKindWithEditor(
sequenceIdentifier,
Multiplicity.Sequence,
sequenceFieldChangeHandler,
(types, other) =>
other.kind.identifier === sequenceIdentifier &&
allowsTreeSchemaIdentifierSuperset(types, other.types),
// TODO: add normalizer/importers for handling ops from other kinds.
new Set([]),
);

const nodeKeyIdentifier = "NodeKey";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export { OptionalChangeset, RegisterId } from "./optionalFieldChangeTypes.js";
export {
optionalChangeHandler,
optionalFieldEditor,
OptionalFieldEditor,
Copy link
Contributor

Choose a reason for hiding this comment

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

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

optionalChangeRebaser,
optionalFieldIntoDelta,
} from "./optionalField.js";
Loading