feat: Add joist-transform-properties to make custom properties lazy (#…

…1061)

* feat: Add joist-transform-properties to lazy custom properties.

* Use __data instead of #orm indirection.

* Re-codegen.

* Add tspc.

* Copy edit.

* Go back to using defineProperty to keep it private.

* Use allFields.

docs/docs/advanced/transform-properties.md

@@ -0,0 +1,34 @@
+---
+title: Properties Transform
+sidebar_position: 35
+---
+
+A key feature of Joist is defining custom properties on your domain model, i.e. `hasOneThrough`, `hasManyThrough`, `hasAsyncProperty`, `hasReactiveField`, etc.
+
+While these properties are powerful, Joist's current API involves defining them as properties directly on each instance of an entity, i.e.:
+
+```ts
+export class Author extends AuthorCodegen {
+  readonly reviews: Collection<Author, BookReview> = hasManyThrough((a) => a.books.reviews);
+}
+```
+
+This means that if you load 1,000 `Author` rows from the database, there will be 1,000 `hasManyThrough` relations initialized, even if this particular endpoint/codepath doesn't end up accessing them.
+
+In the majority of scenarios, this is fine, but when loading ~1,000s of entities, it can become a performance issue.
+
+To address this, Joist provides a [ts-patch](https://github.com/nonara/ts-patch) transformer that will rewrite the fields into lazy getters on the `Author` prototype.
+
+To enable this, add the following to your `tsconfig.json`:
+
+```json
+{
+  "compilerOptions": {
+    "plugins": [
+      { "transform": "joist-transform-properties", "type": "raw" }
+    ]
+  }
+}
+```
+
+And then compile your production code with `tspc` instead of the raw `tsc` command.

docs/docs/faq.md

@@ -6,3 +6,31 @@ position: 10
 ## What databases does Joist support?
 
 Currently only Postgres; see [support other databases](https://github.com/joist-orm/joist-orm/issues/636).
+
+## Why are relations modeled as objects?
+
+In Joist, relations are modeled as wrapper objects, i.e. `Author.books` is not a raw array like `Book[]`, but instead a `Collection<Author, Book[]>` that must have `.load()` and `.get` called on it.
+
+This can initially feel awkward, but it provides a truly type-safe API, given that relations may-or-may not be loaded from the database, and instead are incrementally into memory.
+
+This is often how business logic wants to interact with the domain model--a continual incremental loading of data as needed, as conditional codepaths are executed, instead of an endpoint/program exhaustively knowing up-front exactly what data will be necessary.
+
+If performance is a concern (loading thousands of entities with many custom properties), Joist provides a [ts-patch transform](/docs/advanced/transform-properties) to rewrite the properties as lazy getters in production builds. 
+
+## Why must properties be explicitly typed?
+
+When declaring custom properties on entities, currently the fields must be explicitly typed, i.e. the `Collection<Author, BookReview>` in the following example is required:
+
+```typescript
+export class Author extends AuthorCodegen {
+  readonly reviews: Collection<Author, BookReview> = hasManyThrough((author) => author.books.reviews);
+}
+```
+
+Obviously as TypeScript fans, we'd love to have these field types inferred, and just do `readonly reviews = hasManyThrough`.
+
+Unfortunately, given how interconnected the types of a domain model are, and how sophisticated custom properties can rely on cross-entity typing, attempting to infer the field types quickly leads to the TypeScript compiler failing with cyclic dependency errors, i.e. the `Author`'s fields can only be inferred if `Book` is first typed, but `Book`'s fields can only be inferred if `Author` is first typed.
+
+And adding explicit field types short-circuits these cyclic dependency.
+
+

package.json

@@ -28,7 +28,8 @@
     "packages/tests/esm-misc",
     "packages/tests/number-ids",
     "packages/tests/untagged-ids",
-    "packages/tests/uuid-ids"
+    "packages/tests/uuid-ids",
+    "packages/transform-properties"
   ],
   "devDependencies": {
     "@semantic-release/changelog": "^6.0.3",

packages/codegen/src/generateEntityCodegenFile.ts

@@ -55,7 +55,6 @@ import {
   cleanStringValue,
   failNoIdYet,
   getField,
-  getInstanceData,
   hasLargeMany,
   hasLargeManyToMany,
   hasMany,
@@ -580,8 +579,7 @@ export function generateEntityCodegenFile(config: Config, dbMeta: DbMetadata, me
         } else {
           return code`
             get ${r.fieldName}(): ${r.decl} {
-              const { relations } = ${getInstanceData}(this);
-              return relations.${r.fieldName} ??= ${r.init};
+              return this.__data.relations.${r.fieldName} ??= ${r.init};
             }
           `;
         }

packages/graphql-resolver-utils/src/entityResolver.ts

@@ -79,7 +79,7 @@ export function entityResolver<T extends Entity, A extends Record<string, keyof
     return typeof entityOrId === "string" ? entityOrId : entityOrId.id;
   };
 
-  const primitiveResolvers = Object.values(entityMetadata.fields)
+  const primitiveResolvers = Object.values(entityMetadata.allFields)
     .filter((ormField) => !isPrimaryKeyField(ormField) && !isReferenceField(ormField) && !isCollectionField(ormField))
     .map((ormField) => {
       if (ormField.kind === "primitive" && ormField.derived === "async") {
@@ -90,7 +90,7 @@ export function entityResolver<T extends Entity, A extends Record<string, keyof
       }
     });
 
-  const referenceResolvers: [string, Resolver<any, any, any>][] = Object.values(entityMetadata.fields)
+  const referenceResolvers: [string, Resolver<any, any, any>][] = Object.values(entityMetadata.allFields)
     .filter((ormField) => isReferenceField(ormField))
     .map((ormField) => [
       ormField.fieldName,
@@ -127,7 +127,7 @@ export function entityResolver<T extends Entity, A extends Record<string, keyof
       },
     ]);
 
-  const collectionResolvers: [string, Resolver<T, any, any>][] = Object.values(entityMetadata.fields)
+  const collectionResolvers: [string, Resolver<T, any, any>][] = Object.values(entityMetadata.allFields)
     .filter((ormField) => isCollectionField(ormField))
     .map((ormField) => [
       ormField.fieldName,
@@ -154,8 +154,8 @@ export function entityResolver<T extends Entity, A extends Record<string, keyof
   // Look for non-orm properties on the domain object's prototype
   // and on an instance of the domain object itself to get
   // non-prototype properties like CustomReferences
-  const ignoredKeys = Object.keys(ormResolvers);
-  const customProperties = Object.keys(getProperties(entityMetadata)).filter((n) => !ignoredKeys.includes(n));
+  const alreadyFoundKeys = Object.keys(ormResolvers);
+  const customProperties = Object.keys(getProperties(entityMetadata)).filter((n) => !alreadyFoundKeys.includes(n));
   const customResolvers: [string, Resolver<T, any, any>][] = customProperties.map((key) => [
     key,
     (entity) => {

packages/orm/src/BaseEntity.ts

@@ -14,7 +14,7 @@ import {
 export let currentlyInstantiatingEntity: Entity | undefined;
 
 /**
- * Returns the internal `#orm` tracking field for `entity`.
+ * Returns the internal `__data` tracking field for `entity`.
  *
  * This should be treated as an internal API and may change without notice.
  */
@@ -29,18 +29,18 @@ export function getInstanceData(entity: Entity): InstanceData {
  */
 export abstract class BaseEntity<EM extends EntityManager, I extends IdType = IdType> implements Entity {
   public static getInstanceData(entity: Entity): InstanceData {
-    return (entity as BaseEntity<any>).#orm;
+    return (entity as BaseEntity<any>).__data;
   }
-  readonly #orm!: InstanceData;
+  // We use a protected field so that subclass getters can easily access `this.__data.relations`.
+  protected readonly __data!: InstanceData;
 
   protected constructor(em: EM, optsOrId: any) {
+    const isNew = typeof optsOrId !== "string";
+    const data = new InstanceData(em, (this.constructor as any).metadata, isNew);
+    // This makes it non-enumerable to avoid Jest/recursive things tripping over it
+    Object.defineProperty(this, "__data", { value: data, enumerable: false, writable: false, configurable: false });
     // Only do em.register for em.create-d entities, otherwise defer to hydrate to em.register
-    if (typeof optsOrId === "string") {
-      this.#orm = new InstanceData(em, (this.constructor as any).metadata, false);
-    } else {
-      this.#orm = new InstanceData(em, (this.constructor as any).metadata, true);
-      em.register(this);
-    }
+    if (isNew) em.register(this);
     currentlyInstantiatingEntity = this;
   }
 
@@ -82,15 +82,15 @@ export abstract class BaseEntity<EM extends EntityManager, I extends IdType = Id
    * is no longer new; this only flips to `false` after the `flush` transaction has been committed.
    */
   get isNewEntity(): boolean {
-    return this.#orm.isNewEntity;
+    return this.__data.isNewEntity;
   }
 
   get isDeletedEntity(): boolean {
-    return this.#orm.isDeletedEntity;
+    return this.__data.isDeletedEntity;
   }
 
   get isDirtyEntity(): boolean {
-    return this.#orm.isDirtyEntity;
+    return this.__data.isDirtyEntity;
   }
 
   toString(): string {
@@ -110,7 +110,7 @@ export abstract class BaseEntity<EM extends EntityManager, I extends IdType = Id
   }
 
   public get em(): EM {
-    return this.#orm.em as EM;
+    return this.__data.em as EM;
   }
 
   /**

packages/orm/src/getProperties.ts

@@ -1,4 +1,4 @@
-import { getInstanceData } from "./BaseEntity";
+import { BaseEntity, getInstanceData } from "./BaseEntity";
 import { Entity } from "./Entity";
 import { EntityMetadata } from "./EntityMetadata";
 import { asConcreteCstr } from "./index";
@@ -25,16 +25,29 @@ export function getProperties(meta: EntityMetadata): Record<string, any> {
   if (propertiesCache[key]) {
     return propertiesCache[key];
   }
+
   const instance = getFakeInstance(meta);
+
+  // Mostly for historical reasons, we don't treat known primitives/enums as properties,
+  // i.e. properties were originally meant to be the wrapper objects like `hasOne`,
+  // `hasMany`, `ReactiveField`, etc.
+  //
+  // That said, we've since start leaking other things like getters, regular async methods,
+  // etc., into properties, so that `entityResolver` can pick them up as keys to put into
+  // the GraphQL resolvers. So we should probably just remove this filter and let everything
+  // get returned as properties.
+  const knownPrimitives = Object.values(meta.allFields)
+    .filter((f) => f.kind === "primaryKey" || f.kind === "primitive" || f.kind === "enum")
+    .map((f) => f.fieldName);
+
   propertiesCache[key] = Object.fromEntries(
-    [
-      ...Object.values(meta.allFields)
-        .filter((f) => f.kind !== "primaryKey" && f.kind !== "primitive" && f.kind !== "enum")
-        .map((f) => f.fieldName),
-      ...Object.getOwnPropertyNames(meta.cstr.prototype),
-      ...Object.keys(instance),
-    ]
-      .filter((key) => key !== "constructor" && !key.startsWith("__"))
+    // Recursively looking for ownKeys will find:
+    // - Custom properties set on the instance, like `readonly author: Reference<Author> = hasOneThrough(...)`
+    // - Getters declared within the class like `get initials()`
+    // - Getters auto-created by transform-properties when it lazies `readonly author = hasOneThrough(...)` relations
+    // - Getters declared within the codegen classes like `get books(): Reference<...>`
+    getRecursiveOwnNames(instance)
+      .filter((key) => key !== "constructor" && !key.startsWith("__") && !knownPrimitives.includes(key))
       .map((key) => {
         // Return the value of `instance[key]` but wrap it in a try/catch in case it's
         // a getter that runs code that fails b/c of the dummy state we're in.
@@ -75,3 +88,30 @@ export function getFakeInstance(meta: EntityMetadata): Entity {
     {},
   ));
 }
+
+// These are keys we codegen into `AuthorCodegen` files to get the best typing
+// experience, but really should be treated as BaseEntity keys that we don't
+// need to expose from `getProperties`.
+const ignoredKeys = new Set([
+  "id",
+  "idMaybe",
+  "idTagged",
+  "idTaggedMaybe",
+  "set",
+  "setPartial",
+  "changes",
+  "isSoftDeletedEntity",
+  "load",
+  "populate",
+  "isLoaded",
+  "toJSON",
+]);
+
+// function getRecursiveOwnNames(cstr: MaybeAbstractEntityConstructor<any>): string[] {
+function getRecursiveOwnNames(instance: any): string[] {
+  const keys: string[] = [];
+  for (let curr = instance; curr && curr !== BaseEntity.prototype; curr = Object.getPrototypeOf(curr)) {
+    keys.push(...Object.getOwnPropertyNames(curr));
+  }
+  return keys.filter((k) => !ignoredKeys.has(k));
+}

packages/tests/esm-misc/joist-config.json

@@ -10,5 +10,5 @@
     "createdAt": { "names": ["created_at", "createdAt"], "required": false },
     "updatedAt": { "names": ["updated_at", "updatedAt"], "required": false }
   },
-  "version": "1.160.4"
+  "version": "1.162.0"
 }

packages/tests/esm-misc/src/entities/codegen/AuthorCodegen.ts

@@ -4,7 +4,6 @@ import {
   ConfigApi,
   failNoIdYet,
   getField,
-  getInstanceData,
   hasMany,
   isLoaded,
   loadLens,
@@ -207,7 +206,13 @@ export abstract class AuthorCodegen extends BaseEntity<EntityManager, string> im
   }
 
   get books(): Collection<Author, Book> {
-    const { relations } = getInstanceData(this);
-    return relations.books ??= hasMany(this as any as Author, bookMeta, "books", "author", "authorId", undefined);
+    return this.__data.relations.books ??= hasMany(
+      this as any as Author,
+      bookMeta,
+      "books",
+      "author",
+      "authorId",
+      undefined,
+    );
   }
 }

packages/tests/esm-misc/src/entities/codegen/BookCodegen.ts

@@ -4,7 +4,6 @@ import {
   ConfigApi,
   failNoIdYet,
   getField,
-  getInstanceData,
   hasOne,
   isLoaded,
   loadLens,
@@ -164,7 +163,6 @@ export abstract class BookCodegen extends BaseEntity<EntityManager, string> impl
   }
 
   get author(): ManyToOneReference<Book, Author, never> {
-    const { relations } = getInstanceData(this);
-    return relations.author ??= hasOne(this as any as Book, authorMeta, "author", "books");
+    return this.__data.relations.author ??= hasOne(this as any as Book, authorMeta, "author", "books");
   }
 }

packages/tests/integration/joist-config.json

@@ -94,5 +94,5 @@
     }
   },
   "entitiesDirectory": "./src/entities",
-  "version": "1.160.4"
+  "version": "1.162.0"
 }

packages/tests/integration/package.json

@@ -40,11 +40,13 @@
     "joist-graphql-codegen": "workspace:*",
     "joist-migration-utils": "workspace:*",
     "joist-test-utils": "workspace:*",
+    "joist-transform-properties": "workspace:*",
     "kelonio": "^0.10.0",
     "postgres": "^3.4.4",
     "prettier": "^3.2.5",
     "prettier-plugin-organize-imports": "^3.2.4",
     "superstruct": "0.15.5",
+    "ts-patch": "^3.1.2",
     "tsconfig-paths": "^4.2.0",
     "tsx": "^4.7.2",
     "typescript": "^5.4.5",

packages/tests/integration/src/SingleTableInheritance.test.ts

@@ -253,28 +253,37 @@ describe("SingleTableInheritance", () => {
   it("reports the right properties", () => {
     expect(Object.keys(getProperties(Task.metadata))).toMatchInlineSnapshot(`
      [
+       "typeDetails",
+       "isOld",
+       "isNew",
        "taskTaskItems",
        "tags",
      ]
     `);
     expect(Object.keys(getProperties(TaskNew.metadata))).toMatchInlineSnapshot(`
      [
-       "specialNewAuthor",
        "newTaskTaskItems",
+       "specialNewAuthor",
+       "typeDetails",
+       "isOld",
+       "isNew",
        "taskTaskItems",
        "tags",
      ]
     `);
     expect(Object.keys(getProperties(TaskOld.metadata))).toMatchInlineSnapshot(`
      [
-       "parentOldTask",
+       "commentParentInfo",
        "comments",
        "oldTaskTaskItems",
        "tasks",
+       "parentOldTask",
        "publishers",
+       "typeDetails",
+       "isOld",
+       "isNew",
        "taskTaskItems",
        "tags",
-       "commentParentInfo",
      ]
     `);
   });