Add better documentation for bindgen. (#5025)

* Add documentation for Realm * More documentation and changes. * Remove more links and add docs. * More format consistency. * Add documentation doc. * Minor corrections * Remove discuss * Minor improvements * Update packages/realm/src/OrderedCollection.ts Co-authored-by: Kræn Hansen <[email protected]> Co-authored-by: Kræn Hansen <[email protected]>
realm · Nov 15, 2022 · 0c0f8ab · 0c0f8ab
1 parent 29053ef
commit 0c0f8ab
Show file tree

Hide file tree

Showing 13 changed files with 835 additions and 32 deletions.
diff --git a/packages/realm/CONTRIBUTING.md b/packages/realm/CONTRIBUTING.md
@@ -35,3 +35,6 @@ npx turbo run build
 ```
 
 This will incrementally generate & build any dependencies.
+
+## Documentation
+See `DOCUMENTATION.md` for general principles to follow when adding JSDocs annotations to Realm classes.
diff --git a/packages/realm/DOCUMENTATION.md b/packages/realm/DOCUMENTATION.md
@@ -0,0 +1,81 @@
+For sake of consistency, here are some general principles to follow when writing JSDoc for Realm classes' properties and methods.
+
+### Methods:
+Here are all the tags Realm class methods are likely to have, **listed in the order that they should be described in**.
+- **ALL TAGS**
+    - Should have description start with an uppercase letter. 
+    - Should have description end with a period.
+    - Should **not** have types that are already obvious with TypeScript.
+    - All methods that override built-in JS parent class methods (i.e. `forEach`, `map`) should avoid unnecesary comments (if reasonable) as they will likely already have built-in documentation that is inherited.
+
+- **Description**
+    - Description of the method should go first.
+    - `@link / @linkcode` seems to have often broken behavior, at least in VSCode, so generally from legacy docs it has been replaced with *** {link} *** (i.e. see ***Realm.Collections***) to make what those links bold. In the future one would probably want to replace these with URL links to Realm documentation.
+- `@readonly`?
+    - Include this tag if the field is read-only, this is only relevant for getter methods.
+- `@param {name} {description}`
+    - ✅ `@param `**config**` The configuration of the app.`
+    - ❌ `@param {boolean} config - the config`
+    - Should **not** have a dash after param. name (some of the legacy documentation and is actually ignored by JSDoc but for sake of consistency it would be good to avoid this)
+- `@throws {errorType} {If / When + description}`
+    - ✅ `@throws {Error} If no app id is provided.`
+    - ✅ `@throws {RuntimeError} When Realm closes.`
+    - ❌ `@throws no app id is provided.`
+    - Should have description **start with If or When**.
+    - Should include error type, i.e. `{Error}`, `{TypeError}`.
+- `@returns {description}`
+    - ✅ `@returns The last value or undefined if the list is empty.`
+    - ✅ `@returns `**true**`  if the value exists in the Set, ` **false**` if not.`
+    - ❌ `@returns appId`
+    - For **boolean** return values, use `@returns` \`true\` `if X,` \`false\` `if not.`
+- `@see, etc. ... `
+    - Tags such as `@see` can be decided on case-by-case basis. For example, if `@see` is used instead of description to refer to external documentation, it can be in place of description. If it is used as more of a "if interested, learn more by looking here", `@see` can be included after `@returns`.
+- `@example`
+- `@since`
+    - Kept because of old documenation, not necessarily useful. 
+
+Some examples of annotations following the principles above:
+```ts
+/**
+   * Returns the maximum value of the values in the collection or of the
+   * given property among all the objects in the collection, or `undefined`
+   * if the collection is empty.
+   *
+   * Only supported for int, float, double and date properties. `null` values
+   * are ignored entirely by this method and will not be returned.
+   *
+   * @param property For a collection of objects, the property to take the maximum of.
+   * @throws {Error} If no property with the name exists or if property is not numeric/date.
+   * @returns The maximum value.
+   * @since 1.12.1
+*/
+max(property?: string): number | Date | undefined;
+
+/**
+   * Check for existence of a value in the Set
+   * @param value Value to search for in the Set
+   * @throws {TypeError} If a `value` is not of a type which can be stored in
+   *   the Set, or if an object being added to the Set does not match the
+   *   **object schema** for the Set.
+   * @returns `true` if the value exists in the Set, `false` if not.
+*/
+has(value: T): boolean;
+
+/**
+   * This is the same method as the {@link Collection.values} method.
+   * Its presence makes collections _iterable_, thus able to be used with ES6
+   * {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of `for-of`}
+   * loops,
+   * {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_operator `...`}
+   * spread operators, and more.
+   * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/iterator Symbol.iterator}
+   *   and the {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols#iterable iterable protocol}
+   * @returns Iterable of each value in the collection
+   * @example
+   * for (let object of collection) {
+   *   // do something with each object
+   * }
+   * @since 0.11.0
+   */
+abstract [Symbol.iterator](): Iterator<T>;
+```
diff --git a/packages/realm/src/Collection.ts b/packages/realm/src/Collection.ts
@@ -18,6 +18,22 @@
 
 import { CallbackRegistrator, IllegalConstructorError, Listeners, binding } from "./internal";
 
+/**
+ * Abstract base class containing methods shared by Realm **List**, **Dictionary**, and **Results**.
+ *
+ * A Realm Collection is a homogenous sequence of values of any of the types
+ * that can be stored as properties of Realm objects. A collection can be
+ * accessed in any of the ways that a normal Javascript Array can, including
+ * subscripting, enumerating with `for-of` and so on.
+ *
+ * A Collection always reflect the current state of the Realm. The one exception to this is
+ * when using `for...in` or `for...of` enumeration, which will always enumerate over the
+ * objects which matched the query when the enumeration is begun, even if some of them are
+ * deleted or modified to be excluded by the filter during the enumeration.
+ *
+ * @memberof Realm
+ * @since 0.11.0
+ */
 export abstract class Collection<
   KeyType = unknown,
   ValueType = unknown,
@@ -50,19 +66,83 @@ export abstract class Collection<
     });
   }
 
+  /**
+   * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/keys Array.prototype.keys}
+   * @returns Iterator with all keys in the collection
+   * @since 0.11.0
+   */
   abstract keys(): Iterable<KeyType>;
+
+  /**
+   * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/keys Array.prototype.keys}
+   * @returns Iterator with all values in the collection
+   * @since 0.11.0
+   */
   abstract values(): Iterable<ValueType>;
+
+  /**
+   * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/entries Array.prototype.keys}
+   * @returns Iterator with all key/value pairs in the collection
+   * @since 0.11.0
+   */
   abstract entries(): Iterable<EntryType>;
+
+  /**
+   * This is the same method as the ***Collection.values*** method.
+   * Its presence makes collections _iterable_, thus able to be used with ES6
+   * {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of `for-of`}
+   * loops,
+   * {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_operator `...`}
+   * spread operators, and more.
+   * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/iterator Symbol.iterator}
+   *   and the {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols#iterable iterable protocol}
+   * @returns Iterable of each value in the collection
+   * @example
+   * for (let object of collection) {
+   *   // do something with each object
+   * }
+   * @since 0.11.0
+   */
   abstract [Symbol.iterator](): Iterator<T>;
 
+  /**
+   * Add a listener `callback` which will be called when a **live** collection instance changes.
+   * @param callback A function to be called when changes occur.
+   *   The callback function is called with two arguments:
+   *   - `collection`: the collection instance that changed,
+   *   - `changes`: a dictionary with keys `insertions`, `newModifications`, `oldModifications`
+   *      and `deletions`, each containing a list of indices in the collection that were
+   *      inserted, updated or deleted respectively. `deletions` and `oldModifications` are
+   *      indices into the collection before the change happened, while `insertions` and
+   *      `newModifications` are indices into the new version of the collection.
+   * @throws {Error} If `callback` is not a function.
+   * @example
+   * wines.addListener((collection, changes) => {
+   *  // collection === wines
+   *  console.log(`${changes.insertions.length} insertions`);
+   *  console.log(`${changes.oldModifications.length} oldModifications`);
+   *  console.log(`${changes.newModifications.length} newModifications`);
+   *  console.log(`${changes.deletions.length} deletions`);
+   *  console.log(`new size of collection: ${collection.length}`);
+   * });
+   */
   addListener(callback: ChangeCallbackType): void {
     this.listeners.add(callback);
   }
 
+  /**
+   * Remove the listener `callback` from the collection instance.
+   * @param callback Callback function that was previously
+   *   added as a listener through the **addListener** method.
+   * @throws {Error} If `callback` is not a function.
+   */
   removeListener(callback: ChangeCallbackType): void {
     this.listeners.remove(callback);
   }
 
+  /**
+   * Remove all `callback` listeners from the collection instance.
+   */
   removeAllListeners(): void {
     this.listeners.removeAll();
   }

diff --git a/packages/realm/src/Configuration.ts b/packages/realm/src/Configuration.ts
@@ -116,7 +116,7 @@ export enum ClientResetMode {
 //   /**
 //    * Optional object to configure the setup of an initial set of flexible
 //    * sync subscriptions to be used when opening the Realm. If this is specified,
-//    * {@link Realm.open} will not resolve until this set of subscriptions has been
+//    * ***open*** will not resolve until this set of subscriptions has been
 //    * fully synchronized with the server.
 //    *
 //    * Example:

diff --git a/packages/realm/src/Dictionary.ts b/packages/realm/src/Dictionary.ts
@@ -29,8 +29,6 @@ import {
 const INTERNAL = Symbol("Dictionary#internal");
 const HELPERS = Symbol("Dictionary#helpers");
 
-// TODO: Implement this
-
 type DictionaryChangeSet = {
   deletions: string[];
   modifications: string[];
@@ -98,7 +96,12 @@ const PROXY_HANDLER: ProxyHandler<Dictionary> = {
 };
 
 /**
- * TODO: Make this extends Collection<T> (once that doesn't have a nummeric index accessor)
+ * Instances of this class are returned when accessing object properties whose type is `"Dictionary"`
+ *
+ * Dictionaries behave mostly like a JavaScript object i.e., as a key/value pair
+ * where the key is a string.
+ *
+ * @memberof Realm
  */
 export class Dictionary<T = unknown> extends Collection<string, T, [string, T], [string, T], DictionaryChangeCallback> {
   /**
@@ -213,8 +216,11 @@ export class Dictionary<T = unknown> extends Collection<string, T, [string, T],
   }
 
   /**
-   * Adds given element to the dictionary
+ /**
+   * Add a key with a value or update value if key exists.
+   * @throws {Error} If not inside a write transaction or if value violates type constraints
    * @returns The dictionary
+   * @since 10.6.0
    */
   // @ts-expect-error We're exposing methods in the end-users namespace of keys
   set(element: { [key: string]: T }): this {
@@ -228,7 +234,10 @@ export class Dictionary<T = unknown> extends Collection<string, T, [string, T],
   /**
    * Removes elements from the dictionary, with the keys provided.
    * This does not throw if the keys are already missing from the dictionary.
+   * @param key The key to be removed.
+   * @throws {Error} If not inside a write transaction
    * @returns The dictionary
+   * @since 10.6.0
    */
   // @ts-expect-error We're exposing methods in the end-users namespace of keys
   remove(key: string | string[]): this {
@@ -240,8 +249,11 @@ export class Dictionary<T = unknown> extends Collection<string, T, [string, T],
   }
 
   /**
-   * @returns A plain object for JSON serialization.
-   */
+   * The plain object representation of the Dictionary for JSON serialization.
+   * Use circular JSON serialization libraries such as {@link https://www.npmjs.com/package/@ungap/structured-clone @ungap/structured-clone}
+   * and {@link https://www.npmjs.com/package/flatted flatted} for stringifying Realm entities that have circular structures.
+   * @returns A plain object.
+   **/
   // @ts-expect-error We're exposing methods in the users value namespace
   toJSON(_?: string, cache?: unknown): DefaultObject;
   /** @internal */

diff --git a/packages/realm/src/List.ts b/packages/realm/src/List.ts
@@ -27,6 +27,16 @@ import {
 
 type PartiallyWriteableArray<T> = Pick<Array<T>, "pop" | "push" | "shift" | "unshift" | "splice">;
 
+/**
+ * Instances of this class will be returned when accessing object properties whose type is `"list"`.
+ *
+ * Lists mostly behave like normal Javascript Arrays, except for that they can
+ * only store values of a single type (indicated by the `type` and `optional`
+ * properties of the List), and can only be modified inside a ***write*** transaction.
+ *
+ * @extends Realm.Collection
+ * @memberof Realm
+ */
 export class List<T = unknown> extends OrderedCollection<T> implements PartiallyWriteableArray<T> {
   /**
    * The representation in the binding.
@@ -91,6 +101,11 @@ export class List<T = unknown> extends OrderedCollection<T> implements Partially
     return this.internal.size;
   }
 
+  /**
+   * Remove the **last** value from the list and return it.
+   * @throws {Error} If not inside a write transaction.
+   * @returns The last value or undefined if the list is empty.
+   */
   pop(): T | undefined {
     assert.inTransaction(this.realm);
     const {
@@ -106,8 +121,16 @@ export class List<T = unknown> extends OrderedCollection<T> implements Partially
   }
 
   /**
-   * @param  {T} object
-   * @returns number
+   * Add one or more values to the _end_ of the list.
+   *
+   * @param items Values to add to the list.
+   * @throws {TypeError} If a `value` is not of a type which can be stored in
+   *   the list, or if an object being added to the list does not match the
+   *   ***Realm.ObjectSchema*** for the list.
+   *
+   * @throws {Error} If not inside a write transaction.
+   * @returns A number equal to the new length of
+   *          the list after adding the values.
    */
   push(...items: T[]): number {
     assert.inTransaction(this.realm);
@@ -130,7 +153,9 @@ export class List<T = unknown> extends OrderedCollection<T> implements Partially
   }
 
   /**
-   * @returns T
+   * Remove the **first** value from the list and return it.
+   * @throws {Error} If not inside a write transaction.
+   * @returns The first value or undefined if the list is empty.
    */
   shift(): T | undefined {
     assert.inTransaction(this.realm);
@@ -145,6 +170,17 @@ export class List<T = unknown> extends OrderedCollection<T> implements Partially
     }
   }
 
+  /**
+   * Add one or more values to the _beginning_ of the list.
+   *
+   * @param items Values to add to the list.
+   * @throws {TypeError} If a `value` is not of a type which can be stored in
+   *   the list, or if an object being added to the list does not match the
+   *   ***ObjectSchema*** for the list.
+   * @throws {Error} If not inside a write transaction.
+   * @returns The new ***length*** of
+   *          the list after adding the values.
+   */
   unshift(...items: T[]): number {
     assert.inTransaction(this.realm);
     const {
@@ -163,8 +199,49 @@ export class List<T = unknown> extends OrderedCollection<T> implements Partially
     return internal.size;
   }
 
+  /** TODO
+   * Changes the contents of the list by removing value and/or inserting new value.
+   *
+   * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice Array.prototype.splice}
+   * @param start The start index. If greater than the length of the list,
+   *   the start index will be set to the length instead. If negative, then the start index
+   *   will be counted from the end of the list (e.g. `list.length - index`).
+   * @param deleteCount The number of values to remove from the list.
+   *   If not provided, then all values from the start index through the end of
+   *   the list will be removed.
+   * @returns An array containing the value that were removed from the list. The
+   *   array is empty if no value were removed.
+   */
   splice(start: number, deleteCount?: number): T[];
+  /**
+   * Changes the contents of the list by removing value and/or inserting new value.
+   *
+   * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice Array.prototype.splice}
+   * @param start The start index. If greater than the length of the list,
+   *   the start index will be set to the length instead. If negative, then the start index
+   *   will be counted from the end of the list (e.g. `list.length - index`).
+   * @param deleteCount The number of values to remove from the list.
+   *   If not provided, then all values from the start index through the end of
+   *   the list will be removed.
+   * @param items Values to insert into the list starting at `index`.
+   * @returns An array containing the value that were removed from the list. The
+   *   array is empty if no value were removed.
+   */
   splice(start: number, deleteCount: number, ...items: T[]): T[];
+  /**
+   * Changes the contents of the list by removing value and/or inserting new value.
+   *
+   * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice Array.prototype.splice}
+   * @param start The start index. If greater than the length of the list,
+   *   the start index will be set to the length instead. If negative, then the start index
+   *   will be counted from the end of the list (e.g. `list.length - index`).
+   * @param deleteCount The number of values to remove from the list.
+   *   If not provided, then all values from the start index through the end of
+   *   the list will be removed.
+   * @param items Values to insert into the list starting at `index`.
+   * @returns An array containing the value that were removed from the list. The
+   *   array is empty if no value were removed.
+   */
   splice(start: number, deleteCount?: number, ...items: T[]): T[] {
     // Comments in the code below is copied from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice
     assert.inTransaction(this.realm);