TypeORM lazyload update parent fails on child save

I've had a poke around in your promise-issue repository branch, and discovered a few interesting things:

1. The invalid UPDATE query is being triggered by the initial await aRepo.save(aCreated);, NOT by the insert of B and subsequent foreign key assignment to a.b. Assigning a.b = null before the aRepo.create(a) avoids the issue.

2. Adding the initialization a.b = null; before aRepo.create(a) avoids the unexpected invalid UPDATE; ie:

   const a = new A();
   a.name = "something";
   a.b = null;
   const aCreated = aRepo.create(a);
   await aRepo.save(aCreated);

3. I'm fairly confident that the use of async functions for the inverseSide argument to @OneToOne() (ie. async (o: B) => await o.a)) is incorrect.
   The documentation indicates that this should be just (o: B) => o.a, and the generics on OneToOne also confirm this.
   TypeORM will be resolving the promise BEFORE it passes the value to this function, and an async function returns another Promise rather than the correct property value.

4. I've also just noticed that you're passing an instance of class A to aRepo.create(). This isn't necessary; you can pass your instance directly to aRepo.save(a). The Repository.create() simply copies the values from the provided object into a new instance of the entity class. It also appears that .create() creates promises when they don't already exist. This might actually be the cause of this issue; logging aCreated just before aRepo.save(aCreated) is called shows that the promise isn't resolved.
   In fact, removing the aRepo.create(a) step (and changing the save to await aRepo.save(a); also seems to avoid this issue. Perhaps Repository<T>.create() is handling lazy load properties differently when its argument is already instanceof T? I'll look into this.

I also tried upgrading the typeorm package to typeorm@next (0.3.0-alpha.12) but the issue still appears to exist there.

I've just noticed you've already logged a GitHub issue for this; I'll look at creating a test case to demonstrate over the next few days.

I hope this is enough to answer your question!

Update

After some further code tracing, it appears that item 4) from the list above is the reason for this issue.

In RelationLoader.enableLazyLoad(), TypeORM overloads lazy property accessors on @Entity instances with its own getter and setter - e.g. Object.defineProperty(A, 'b', ...). The overloaded property accessors load and cache the related B record, returning Promise<B>.

Repository.create() iterates all relations for the created entity and - when an object is provided - builds a new related object from the provided values. However, this logic doesn't account for a Promise object, and tries to build a related entity directly from the Promise's properties.

So in your case above, aRepo.create(a) builds a new A, iterates the A relations (ie. b), and builds an empty B from the Promise on a.b. The new B doesn't have any properties defined because Promise instances don't share any properties B. Then because there is no id specified, the foreign key name and value are not defined for aRepo.save(), leading to the error you've encountered.

So simply passing a directly to aRepo.save() and removing the aRepo.create(a) step seems the correct course of action in this case.

This is an issue that should be fixed however - but I don't think it's an easy fix since this really needs for Repository.create() to be able to await the Promise; which not achievable at present since Repository.create() is not async.

Following up on @Timshel's fantastic answer (and attempt at fixing the underlying issue in typeorm itself).

For those finding there way here who are looking for a workaround in lieu of https://github.com/typeorm/typeorm/pull/2902 being merged, i think i've figured something out (assuming you're using the ActiveRecord pattern with typeorm). First to summarize, as most of this information is absent from documentation and needs to be pieced together from various github issues/this SO question:

As pointed out here and on the corresponding issue, when using create passing in a Promise for a lazy loaded relation field simply does not work despite the type signature of that function demanding otherwise (and despite the documentation suggesting that lazy loaded fields should be wrapped in Promise.resolve's for saving purposes). What does seem to work according to @Timshel's comment in the aforementioned PR is:

nasty TypeScript type casts when assigning object literals to lazy-load properties

What this means is that with the create method, if you pass in a plain entity object (instead of a Promise containing said object) for one of these lazy loaded fields typeorm will actually set this value correctly and you will be able to save. You'll even magically get a promise when accessing this field later on. The above quote mentions that you can take advantage of this by force casting your entity objects as Promise's before passing them into create. But this needs to be done on a case by case basis and if you ever accidentally obey the type signature rather than force casting, you're going to have an unexpected result at runtime. Wouldn't it be great if we could correct this type signature to get the compiler to only yell at us when we use this function in a way that won't work? We can, heres how :).

import {
  BaseEntity,
  DeepPartial,
  ObjectType,
} from 'typeorm';

/**
 * Conditional type that takes a type and maps every property which is
 * a Promise to the unwrapped value of that Promise. Specifically to correct the type
 * of typeorm's create method. Using this otherwise would likely be incredibly unwise.
 *
 * For example this type:
 * {
 *   hey: number,
 *   thing: Promise<ThingEntity>,
 *   sup: string
 * }
 *
 * gets mapped to:
 * {
 *   hey: number,
 *   thing: ThingEntity,
 *   sup: string
 * }
 *
 */
type DePromisifyValue<T> = T extends Promise<infer U> ? U : T;
type DePromisifyObject<T> = T extends object
  ? { [K in keyof T]: DePromisifyValue<T[K]> }
  : T;

export abstract class CommonEntity extends BaseEntity {
  static create<T extends CommonEntity>(
    this: ObjectType<T>,
    entityLike?: DeepPartial<DePromisifyObject<T>>
  ): T {
    if (!entityLike) {
      return super.create<T>();
    }
    return super.create<T>(entityLike as DeepPartial<T>);
  }
}

What this does is define an overridden version of this create method which accepts the same object argument as the original create method, except where any Promise fields are there unwrapped versions (ie myLazyLoadedUser: Promise<UserEntity> becomes myLazyLoadedUser: UserEntity). It then passes that into the original create method and force casts it to the old version with all the Promise fields, the way that BaseEntity likes (or lies about liking that is). Force casting at some point can't be avoided without the issue being fixed within typeorm itself, but this solution only requires force casting in one central place where we can be confident we're doing the right thing. Just extend this CommonEntity (call it whatever you want) instead of BaseEntity and the create method will require the correct type of you. No need to wrap your values in Promise.resolve. And the entity returned from it will still have those lazy loaded fields with their original Promise types.

Note: I haven't handled the type signature for create where an array of objects is passed in. I've no need for this myself, but i'm sure the type for that with the same approach can be worked out with sufficient effort.