Improve types for Y.Map's public interface

src/types/YMap.js

@@ -20,12 +20,37 @@ import * as iterator from 'lib0/iterator'
 
 /**
  * @template T
+ * @typedef {Extract<keyof T, string>} StringKey<T>
+ *
+ * Like keyof, but guarantees the returned type is a subset of string.
+ * `keyof` will include number and Symbol even if the input type requires string keys.
+ */
+
+/**
+ * @template T
+ * @typedef {readonly {
+ *            [K in StringKey<T>]: [K, T[K]];
+ *          }[StringKey<T>][]} EntriesOf<T>
+ *
+ * Converts an object schema into a readonly array containing valid key-value pairs.
+ */
+
+/**
+ * This works around some weird JSDoc+TS circular reference issues: https://github.com/microsoft/TypeScript/issues/46369
+ * @typedef {boolean|null|string|number|Uint8Array|JsonArray|JsonObject} Json
+ * @typedef {Json[]} JsonArray
+ * @typedef {{ [key: string]: Json }} JsonObject
+ * @typedef {Json|AbstractType<any>|Doc} MapValue
+ */
+
+/**
+ * @template {Record<string, MapValue>} T
  * @extends YEvent<YMap<T>>
  * Event that describes the changes on a YMap.
  */
 export class YMapEvent extends YEvent {
   /**
-   * @param {YMap<T>} ymap The YArray that changed.
+   * @param {YMap<T>} ymap The YMap that changed.
    * @param {Transaction} transaction
    * @param {Set<any>} subs The keys that changed.
    */
@@ -36,21 +61,21 @@ export class YMapEvent extends YEvent {
 }
 
 /**
- * @template MapType
+ * @template {Record<string, MapValue>} MapType
  * A shared Map implementation.
  *
  * @extends AbstractType<YMapEvent<MapType>>
- * @implements {Iterable<[string, MapType]>}
+ * @implements {Iterable<[StringKey<MapType>, MapType[StringKey<MapType>]]>}
  */
 export class YMap extends AbstractType {
   /**
    *
-   * @param {Iterable<readonly [string, any]>=} entries - an optional iterable to initialize the YMap
+   * @param {EntriesOf<MapType>=} entries - an optional iterable to initialize the YMap
    */
   constructor (entries) {
     super()
     /**
-     * @type {Map<string,any>?}
+     * @type {Map<StringKey<MapType>, MapType[StringKey<MapType>]>?}
      * @private
      */
     this._prelimContent = null
@@ -74,7 +99,7 @@ export class YMap extends AbstractType {
    */
   _integrate (y, item) {
     super._integrate(y, item)
-    ;/** @type {Map<string, any>} */ (this._prelimContent).forEach((value, key) => {
+    this._prelimContent?.forEach((value, key) => {
       this.set(key, value)
     })
     this._prelimContent = null
@@ -114,11 +139,11 @@ export class YMap extends AbstractType {
   /**
    * Transforms this Shared Type to a JSON object.
    *
-   * @return {Object<string,any>}
+   * @return {Partial<MapType>}
    */
   toJSON () {
     /**
-     * @type {Object<string,MapType>}
+     * @type {any}
      */
     const map = {}
     this._map.forEach((item, key) => {
@@ -151,7 +176,7 @@ export class YMap extends AbstractType {
   /**
    * Returns the values for each element in the YMap Type.
    *
-   * @return {IterableIterator<MapType>}
+   * @return {IterableIterator<MapType[keyof MapType]>}
    */
   values () {
     return iterator.iteratorMap(createMapIterator(this._map), /** @param {any} v */ v => v[1].content.getContent()[v[1].length - 1])
@@ -160,7 +185,7 @@ export class YMap extends AbstractType {
   /**
    * Returns an Iterator of [key, value] pairs
    *
-   * @return {IterableIterator<[string, MapType]>}
+   * @return {IterableIterator<[StringKey<MapType>, MapType[StringKey<MapType>]]>}
    */
   entries () {
     return iterator.iteratorMap(createMapIterator(this._map), /** @param {any} v */ v => /** @type {any} */ ([v[0], v[1].content.getContent()[v[1].length - 1]]))
@@ -169,20 +194,20 @@ export class YMap extends AbstractType {
   /**
    * Executes a provided function on once on every key-value pair.
    *
-   * @param {function(MapType,string,YMap<MapType>):void} f A function to execute on every element of this YArray.
+   * @param {function(MapType[StringKey<MapType>],StringKey<MapType>,YMap<MapType>):void} f A function to execute on every element of this YArray.
    */
   forEach (f) {
     this._map.forEach((item, key) => {
       if (!item.deleted) {
-        f(item.content.getContent()[item.length - 1], key, this)
+        f(item.content.getContent()[item.length - 1], /** @type {StringKey<MapType>} */ (key), this)
       }
     })
   }
 
   /**
    * Returns an Iterator of [key, value] pairs
    *
-   * @return {IterableIterator<[string, MapType]>}
+   * @return {IterableIterator<[StringKey<MapType>, MapType[StringKey<MapType>]]>}
    */
   [Symbol.iterator] () {
     return this.entries()
@@ -204,17 +229,18 @@ export class YMap extends AbstractType {
   }
 
   /**
+   * @template {StringKey<MapType>} Key
+   * @template {MapType[Key]} Value
    * Adds or updates an element with a specified key and value.
-   * @template {MapType} VAL
    *
-   * @param {string} key The key of the element to add to this YMap
-   * @param {VAL} value The value of the element to add
-   * @return {VAL}
+   * @param {Key} key The key of the element to add to this YMap
+   * @param {Value} value The value of the element to add
+   * @return {Value}
    */
   set (key, value) {
     if (this.doc !== null) {
       transact(this.doc, transaction => {
-        typeMapSet(transaction, this, key, /** @type {any} */ (value))
+        typeMapSet(transaction, this, key, value)
       })
     } else {
       /** @type {Map<string, any>} */ (this._prelimContent).set(key, value)
@@ -223,13 +249,14 @@ export class YMap extends AbstractType {
   }
 
   /**
+   * @template {StringKey<MapType>} Key
    * Returns a specified element from this YMap.
    *
-   * @param {string} key
-   * @return {MapType|undefined}
+   * @param {Key} key
+   * @return {MapType[Key]|undefined}
    */
   get (key) {
-    return /** @type {any} */ (typeMapGet(this, key))
+    return /** @type {MapType[Key]|undefined} */ (typeMapGet(this, key))
   }
 
   /**

src/utils/Doc.js

@@ -251,14 +251,14 @@ export class Doc extends Observable {
   }
 
   /**
-   * @template T
+   * @template {Record<string, import("../internals.js").MapValue>} T
    * @param {string} [name]
    * @return {YMap<T>}
    *
    * @public
    */
   getMap (name = '') {
-    return /** @type {YMap<T>} */ (this.get(name, YMap))
+    return /** @type {YMap<T>} */ /** @type {any} */ (this.get(name, YMap))
   }
 
   /**

tests/doc.tests.js

@@ -114,25 +114,26 @@ export const testSubdoc = _tc => {
     doc.on('subdocs', subdocs => {
       event = [Array.from(subdocs.added).map(x => x.guid), Array.from(subdocs.removed).map(x => x.guid), Array.from(subdocs.loaded).map(x => x.guid)]
     })
+    /** @type {Y.Map<{ a: Y.Doc; b: Y.Doc, c: Y.Doc; }>} */
     const subdocs = doc.getMap('mysubdocs')
     const docA = new Y.Doc({ guid: 'a' })
     docA.load()
     subdocs.set('a', docA)
     t.compare(event, [['a'], [], ['a']])
 
     event = null
-    subdocs.get('a').load()
+    subdocs.get('a')?.load()
     t.assert(event === null)
 
     event = null
-    subdocs.get('a').destroy()
+    subdocs.get('a')?.destroy()
     t.compare(event, [['a'], ['a'], []])
-    subdocs.get('a').load()
+    subdocs.get('a')?.load()
     t.compare(event, [[], [], ['a']])
 
     subdocs.set('b', new Y.Doc({ guid: 'a', shouldLoad: false }))
     t.compare(event, [['a'], [], []])
-    subdocs.get('b').load()
+    subdocs.get('b')?.load()
     t.compare(event, [[], [], ['a']])
 
     const docC = new Y.Doc({ guid: 'c' })
@@ -156,7 +157,9 @@ export const testSubdoc = _tc => {
     Y.applyUpdate(doc2, Y.encodeStateAsUpdate(doc))
     t.compare(event, [['a', 'a', 'c'], [], []])
 
-    doc2.getMap('mysubdocs').get('a').load()
+    /** @type {Y.Map<Record<string, Y.Doc>>} */
+    const mysubdocs = doc2.getMap('mysubdocs')
+    mysubdocs.get('a')?.load()
     t.compare(event, [[], [], ['a']])
 
     t.compare(Array.from(doc2.getSubdocGuids()), ['a', 'c'])

tests/test-types.ts

@@ -0,0 +1,92 @@
+/**
+ * This file serves to validate the public TypeScript API for YJS.
+ *
+ * It is not included in `npm run lint` or any other automated type checking, but can be used
+ * by those working on YJS types to ensure that the public-facing type interface remains valid.
+ *
+ * Any lines which are supposed to demonstrate statements that _would_ generate type errors
+ * should be clearly marked with the type error that is expected to result, to provide a
+ * negative test case.
+ */
+
+import * as Y from "../dist/src/index";
+
+/*
+ * Typed maps
+ *
+ * - Key names are autocompleted in first parameter of `get` and `set`.
+ * - `MapType` value types are constrained to valid Y.Map contents.
+ */
+type MyType = {
+  foo: string;
+  bar: number | null;
+  baz?: boolean;
+};
+
+// Constructor argument keys & values are typechecked, and keys are autocompleted.
+// Multiple items for each key and partial initialization are allowed.
+const map = new Y.Map<MyType>([
+  ["foo", ""],
+  ["foo", "even better"],
+  // ERROR: Type '["baz", number]' is not assignable to type '["foo", string] | ["bar", number | null] | ["baz", boolean | undefined]'.
+  ["baz", 3],
+]);
+
+// Entries are still allowed to be omitted, so get() still returns <type> | undefined.
+const defaultMap = new Y.Map<MyType>();
+
+// `json` has a type of `Partial<MyType>`
+const json = defaultMap.toJSON();
+
+// string | undefined
+const fooValue = map.get("foo");
+// literal "hi" (string)
+const fooSet = map.set("foo", "hi");
+// number | null | undefined
+const barValue = map.get("bar");
+// ERROR: Argument of type '"hi"' is not assignable to parameter of type 'number | null'.
+const barSet = map.set("bar", "hi");
+// ERROR: Argument of type '"bomb"' is not assignable to parameter of type 'keyof MyType'.
+const missingGet = map.get("bomb");
+// Escape hatch: get<any>()
+const migrateGet = map.get<any>("extraneousKey");
+
+// ERROR: Type '<type>' does not satisfy the constraint 'Record<string, MapValue>'.
+const invalidMap = new Y.Map<{ invalid: () => void }>();
+const invalidMap2 = new Y.Map<{ invalid: Blob }>();
+// Arbitrarily complex valid types are still allowed
+type ComplexType = {
+  n: null;
+  b: boolean;
+  s: string;
+  i: number;
+  u: Uint8Array;
+  a: null | boolean | string | number | Uint8Array[];
+};
+const complexValidType = new Y.Map<
+  ComplexType & { nested: ComplexType & { deeper: ComplexType[] } }
+>();
+
+/*
+ * Default behavior
+ *
+ * Provides basic typechecking over the range of possible map values.
+ */
+const untyped = new Y.Map();
+
+// MapValue | undefined
+const boop = untyped.get("default");
+// Still validates value types: ERROR: Argument of type '() => string' is not assignable to parameter of type 'MapValue'.
+const moop = untyped.set("anything", () => "whoops");
+
+/*
+ * `any` maps (bypass typechecking)
+ */
+const anyMap = new Y.Map<any>();
+
+// any
+const fooValueAny = anyMap.get("foo");
+// literal "hi" (string)
+const fooSetAny = anyMap.set("foo", "hi");
+// Allowed because `any` unlocks cowboy mode
+const barSetAny = anyMap.set("bar", () => "hi");

tests/undo-redo.tests.js

@@ -369,6 +369,7 @@ export const testUndoUntilChangePerformed = _tc => {
   const yMap = new Y.Map()
   yMap.set('hello', 'world')
   yArray.push([yMap])
+  /** @type {Y.Map<{ key: string }>} */
   const yMap2 = new Y.Map()
   yMap2.set('key', 'value')
   yArray.push([yMap2])
@@ -382,7 +383,7 @@ export const testUndoUntilChangePerformed = _tc => {
   Y.transact(doc2, () => yArray2.delete(0), doc2.clientID)
   undoManager2.undo()
   undoManager.undo()
-  t.compareStrings(yMap2.get('key'), 'value')
+  t.compareStrings(yMap2.get('key') || '', 'value')
 }
 
 /**
@@ -447,6 +448,7 @@ export const testUndoNestedUndoIssue = _tc => {
  */
 export const testConsecutiveRedoBug = _tc => {
   const doc = new Y.Doc()
+  /** @type {Y.Map<Record<string, Y.Map<any>>>} */
   const yRoot = doc.getMap()
   const undoMgr = new Y.UndoManager(yRoot)
 
@@ -480,7 +482,7 @@ export const testConsecutiveRedoBug = _tc => {
   t.compare(yRoot.get('a'), undefined)
 
   undoMgr.redo() // x=0, y=0
-  yPoint = yRoot.get('a')
+  yPoint = yRoot.get('a') || new Y.Map()
 
   t.compare(yPoint.toJSON(), { x: 0, y: 0 })
   undoMgr.redo() // x=100, y=100