feat(entity-generator): allow custom types for scalar relations

[boenrobot]

This is done by fully refactoring type inference in general,
to take advantage of "runtimeType" in the prop,
combined with more cross checks to add IType where needed.

Also removed the accidental Embedded decorator in EntitySchema.

---

Using custom types for scalar relations was sort of possible even before this, but it has always been more hacky than anything else. With this, it is actually encoded as an expected possibility, and added to the test for hooks. Users who want to use custom types will need to handle their resolution in the fileName function. Same as if those identifiers were referring to an entity.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.74%. Comparing base (477334e) to head (6a6b23b).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5435   +/-   ##
=======================================
  Coverage   99.74%   99.74%           
=======================================
  Files         260      260           
  Lines       17990    18026   +36     
  Branches     4374     4395   +21     
=======================================
+ Hits        17944    17980   +36     
  Misses         46       46           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[B4nan]

where is this CustomBooleanType defined?

[boenrobot]

As far as the entity generator is concerned, that's none of its business 😆. The extension author is expected to have defined it previously, specify the reference to it in an extension hook, and override fileName to resolve the referenced class.

In this case, it's not actually defined (just referenced in the extension hook), and the import is resolved to being in the same folder as the entities.

I expect in practice that people would define their custom prefix/suffix, and override fileName to resolve classes with that prefix/suffix to another folder... So something like fileName: (className) => className.endsWith('Type') ? `../types/${className}` : className .

The referenced class may also be defined later in a larger script, in which entity generation is just the biggest step.

[B4nan]

can we at least generate a stub for this? like export type CustomBooleanType = boolean, which users can replace?

as this feels like the only thing that the generator would output and wouldn't exist

[boenrobot]

Hm... now that you showed this specific stub, I'm thinking there's actually a different can of worms I haven't dealt with... In fact, it's fair to say I didn't think the whole thing through...

We have...

1. Type only imports for non-core imports in the prop declaration... The one in the test was meant to be imported and used as such. Custom shapes for plain JSON values (as opposed to embedded JSON values) are perhaps a more realistic example of something that users would want to do, and would still end up having to import as a type import. My CustomBooleanType example was instead an example of the second use case for type only scalar imports - a branded type (in this case, a branded boolean, though branded integers are perhaps a more frequent use case).
2. IType annotated custom types. The above does not qualify as such, but what about types that do? Things like "an email type" for example that validate on input. The meta data would have the type class, but the type class is not what should be in the prop definition (just the prop options instead).

The scope of this PR suddenly increased a bit, but I guess better to address at least one of those two things, if not both...

I'm thinking of trying to do a dynamic import of the referenced identifier (when the ref is a scalar non-enum of a non-lib type...) after letting fileName dictate the import path... if the file does not exist, or the identifier does not exist in it, or exists, but is not a mapped type, assume it is a type only import, and output it as such (kind of like now, except CustomBooleanType would be imported as type). Otherwise, check runtimeType and compareAsType() to determine the value of the prop type. If they are the same, use that for the prop declaration. If not, use IType<${runtimeType}, ${compareAsType}>, and add an import for runtimeType (if it's a non-lib type, which is the typical case).

Even if the above approach works, I don't see how to get the serialization type out of the Type, but then again, there's also 0 examples of using it anywhere (not used in any test, not in docs...), so I guess that's fine for now.

[B4nan]

after letting fileName dictate the import path

This bit was also pretty confusing to me, if we expect people to use this to override import paths, we need to document it somewhere. And I am not so sure it's a good idea.

I feel like you are building something without a clear understanding of how the end users would do stuff. As I mentioned many times, entity generator wasn't meant as a schema-first approach (it was more of a helper instead), which I think you are trying to build. So I would first think about how are people supposed to combine it with their own code, and only then think about those cases. And of course, this all needs to be clearly documented, making code changes without that won't help much. Feels like you are going in the wrong direction, I would design this from the top to bottom.

[boenrobot]

OK, I implemented a completely different approach, which however requires a big change in what the generator produces overall...

Basically, we're taking advantage of there being both runtimeType and type properties, with the user not necessarily touching all of them in an extension, and either of those not necessarily matching the mapping from columnType. And also, the name of the type in the "type" option is determined from the first matching entry in the "types" map (or "unknown" if no match).

This produces a very different output in pretty much all entity generator tests though... The value of "type", where present, is now the type in the types map, rather than the runtime type, and this change also comes with the perk of columnType being needed less often.

I've added to the docs a very (perhaps overly) detailed explanation on how types are handled, to aid meta data extensions, with some examples. Let me know if I should copy it over to 6.2, or if, due to this being somewhat of a breaking change for extensions (due to "runtimeType" now being used in many places instead of "type"...), maybe not, and keep this only for the "next" (6.3?) version.

Even with this more comprehensive change, the extra type and runtimeType classes are imported with the fileName config being the way to adjust the path, with the generator making no assumptions or requirements about those extensions' existence, and the docs specifically mention this.

If you think using fileName for this is a mistake, then maybe it's a time for a dedicated generator naming strategy? One to move the getEntityName() and columnNameToProperty() into, and add to it additional methods like getEntityImport() (equivalent of current use of fileName), getTypeImport(), getRuntimeTypeImport() (defaulting to the same implementation)... I mean, sure, those could exist into AbstractNamingStrategy too, but that would mean many new methods used only during entity generation, which sounds very counter "single responsibility principle".