fix: Add tests for previous reactive hint fix, with more fixes.

This adds a test to reproduce the prior "walk through a different
subtype that does not have the relation" error, as well as applies
the fix to m2o and m2m handling as well.

There is also a related fix that runtime filtering of the type filter,
i.e. `publisher@Publisher` should pass for subtypes of `Publisher` like
either a `LargePublisher` or `SmallPublisher`, and previously we were
only exact-matching on the type name.

packages/orm/src/EntityMetadata.ts

@@ -215,6 +215,11 @@ export function getBaseSelfAndSubMetas(meta: EntityMetadata): EntityMetadata[] {
   return [...meta.baseTypes, meta, ...meta.subTypes];
 }
 
+export function getSubMetas(meta: EntityMetadata): EntityMetadata[] {
+  // We should do recursion at some point
+  return meta.subTypes;
+}
+
 export function getBaseMeta(meta: EntityMetadata): EntityMetadata {
   if (!meta.baseType) {
     return meta;

packages/orm/src/reactiveHints.ts

@@ -1,6 +1,16 @@
 import { Entity } from "./Entity";
 import { FieldsOf, MaybeAbstractEntityConstructor, RelationsOf, getEmInternalApi } from "./EntityManager";
-import { EntityMetadata, getBaseAndSelfMetas, getMetadata } from "./EntityMetadata";
+import {
+  EntityMetadata,
+  ManyToManyField,
+  ManyToOneField,
+  OneToManyField,
+  OneToOneField,
+  PolymorphicFieldComponent,
+  getBaseAndSelfMetas,
+  getMetadata,
+  getSubMetas,
+} from "./EntityMetadata";
 import { Changes, FieldStatus, ManyToOneFieldStatus } from "./changes";
 import { isChangeableField } from "./fields";
 import { getProperties } from "./getProperties";
@@ -146,9 +156,10 @@ export function reverseReactiveHint<T extends Entity>(
           if (!isReadOnly) {
             fields.push(field.fieldName);
           }
+          const otherFieldName = maybeAddTypeFilterSuffix(meta, field);
           return reverseReactiveHint(rootType, field.otherMetadata().cstr, subHint, undefined, false).map(
             ({ entity, fields, path }) => {
-              return { entity, fields, path: [...path, field.otherFieldName] };
+              return { entity, fields, path: [...path, otherFieldName] };
             },
           );
         }
@@ -161,21 +172,18 @@ export function reverseReactiveHint<T extends Entity>(
           return field.components.flatMap((comp) => {
             return reverseReactiveHint(rootType, comp.otherMetadata().cstr, subHint, undefined, false).map(
               ({ entity, fields, path }) => {
-                return { entity, fields, path: [...path, comp.otherFieldName] };
+                const otherFieldName = maybeAddTypeFilterSuffix(meta, comp);
+                return { entity, fields, path: [...path, otherFieldName] };
               },
             );
           });
         }
         case "m2m": {
-          const otherField =
-            field.otherMetadata().allFields[field.otherFieldName] ??
-            fail(`No field ${field.otherMetadata().type}.${field.otherFieldName}`);
-          const otherFieldName =
-            otherField.kind === "poly" ? `${field.otherFieldName}@${meta.type}` : field.otherFieldName;
           // While o2m and o2o can watch for just FK changes by passing `reactForOtherSide` (the FK lives in the other
           // table), for m2m reactivity we push the collection name into the reactive hint, because it's effectively
           // "the other/reverse side", and JoinRows will trigger it explicitly instead of `setField` for non-m2m keys.
           fields.push(field.fieldName);
+          const otherFieldName = maybeAddTypeFilterSuffix(meta, field);
           return reverseReactiveHint(
             rootType,
             field.otherMetadata().cstr,
@@ -191,10 +199,7 @@ export function reverseReactiveHint<T extends Entity>(
         case "o2m":
         case "o2o": {
           const isOtherReadOnly = field.otherMetadata().allFields[field.otherFieldName].immutable;
-          const otherFieldName =
-            field.otherMetadata().allFields[field.otherFieldName].kind === "poly" || meta.baseType
-              ? `${field.otherFieldName}@${meta.type}`
-              : field.otherFieldName;
+          const otherFieldName = maybeAddTypeFilterSuffix(meta, field);
           // This is not a field, but we want our reverse side to be reactive, so pass reactForOtherSide
           return reverseReactiveHint(
             rootType,
@@ -252,6 +257,21 @@ export function reverseReactiveHint<T extends Entity>(
   ];
 }
 
+function maybeAddTypeFilterSuffix(
+  meta: EntityMetadata,
+  field: ManyToOneField | OneToManyField | ManyToManyField | OneToOneField | PolymorphicFieldComponent,
+): string {
+  // If we're Foo, and the other field (which we'll traverse back from at runtime) is actually
+  // a poly FK pointing back to multiple `owner=Foo | Bar | Zaz`, add a suffix of `@Foo` so
+  // that any runtime traversal that hit `owner` and see a `Bar | Zaz` will stop.
+  const nextFieldIsPoly = field.otherMetadata().allFields[field.otherFieldName].kind === "poly";
+  // If we're a SubType, and the other field points to our base type, assume there might be
+  // SubType-specific fields in the path so far, so tell the other side to only walk back through
+  // us if its value is actually a SubType.
+  const nextFieldPointsToBaseType = !!meta.baseType && field.otherMetadata().cstr !== meta.cstr;
+  return nextFieldIsPoly || nextFieldPointsToBaseType ? `${field.otherFieldName}@${meta.type}` : field.otherFieldName;
+}
+
 /**
  * Walks `reverseHint` for every entity in `entities`.
  *
@@ -264,11 +284,11 @@ export async function followReverseHint(entities: Entity[], reverseHint: string[
   // And "walk backwards" through the reverse hint
   while (paths.length) {
     const path = paths.shift()!;
-    const [fieldName, viaPolyType] = path.split("@");
+    const [fieldName, viaType] = path.split("@");
     // The path might touch either a reference or a collection
     const entitiesOrLists = await Promise.all(
       current.flatMap((c: any) => {
-        const currentValuePromise = maybeLoadedPoly(c[fieldName].load(), viaPolyType);
+        const currentValuePromise = maybeApplyTypeFilter(c[fieldName].load(), viaType);
         // If we're going from Book.author back to Author to re-validate the Author.books collection,
         // see if Book.author has changed, so we can re-validate both the old author's books and the
         // new author's books.
@@ -279,7 +299,7 @@ export async function followReverseHint(entities: Entity[], reverseHint: string[
         if (isReference && changed && changed.hasUpdated && changed.originalValue) {
           return [
             currentValuePromise,
-            maybeLoadedPoly((changed as ManyToOneFieldStatus<any>).originalEntity, viaPolyType),
+            maybeApplyTypeFilter((changed as ManyToOneFieldStatus<any>).originalEntity, viaType),
           ];
         }
         if (isManyToMany) {
@@ -374,14 +394,34 @@ export interface ReactiveTarget {
   path: string[];
 }
 
-async function maybeLoadedPoly(loadPromise: Promise<Entity>, viaPolyType: string | undefined) {
-  if (viaPolyType) {
-    const loaded: Entity = await loadPromise;
-    return loaded && getMetadata(loaded).type === viaPolyType ? loaded : undefined;
+function maybeApplyTypeFilter(loadPromise: Promise<Entity | Entity[]>, viaType: string | undefined) {
+  if (viaType) {
+    return loadPromise.then((loaded) => {
+      if (Array.isArray(loaded)) {
+        return loaded.filter((e) => isTypeOrSubType(e, viaType));
+      } else if (loaded && isTypeOrSubType(loaded, viaType)) {
+        return loaded;
+      } else {
+        return undefined;
+      }
+    });
   }
   return loadPromise;
 }
 
+/** Handle `viaType` filtering with subtype awareness. */
+function isTypeOrSubType(entity: Entity, typeName: string): boolean {
+  const meta = getMetadata(entity);
+  // Easy check for the name is the same
+  if (meta.type === typeName) return true;
+  // Otherwise see if the entity is a subtype of the typeName, i.e. if our poly/type
+  // filter is `@Publisher`, and we're a `SmallPublisher`, that's valid to traverse.
+  for (const other of getSubMetas(meta)) {
+    if (other.type === typeName) return true;
+  }
+  return false;
+}
+
 export function isPolyHint(key: string): boolean {
   return key.includes("@");
 }

packages/tests/integration/src/EntityManager.reactiveRules.test.tsx

@@ -8,6 +8,7 @@ import {
   newBook,
   newBookReview,
   newPublisher,
+  newSmallPublisher,
   newTag,
   SmallPublisher,
   Tag,
@@ -209,6 +210,14 @@ describe("EntityManager.reactiveRules", () => {
     expect(b.reviewsRuleInvoked).toBe(3);
   });
 
+  it.withCtx("skips traversing through subtype-only relations", async ({ em }) => {
+    // Given we trigger an Image -> publishers -> critics (only exists on LargePublishers)
+    newSmallPublisher(em, { images: [{}] });
+    // Then it does not blow up (and we can't assert against the Critic rule having executed
+    // b/c the scenario is that SmallPublishers don't have the `critics` relation)
+    await em.flush();
+  });
+
   it.withCtx("creates the right reactive rules", async () => {
     const fn = expect.any(Function);
     expect(getReactiveRules(Author)).toMatchObject([

packages/tests/integration/src/entities/Critic.ts

@@ -2,5 +2,5 @@ import { CriticCodegen, criticConfig as config } from "./entities";
 
 export class Critic extends CriticCodegen {}
 
-// remove once you have actual rules/hooks
-config.placeholder();
+/** For testing walking through subtype relations. */
+config.addRule({ favoriteLargePublisher: "images" }, () => {});