fix: edge deletion behavior cyclic import structure

[wschurman]

Why

The way this was created led to import cycles in top level module exports, causing them to resolve as undefined. (as described in https://medium.com/visual-development/how-to-fix-nasty-circular-dependency-issues-once-and-for-all-in-javascript-typescript-a04c987cf0de)

How

Make one end of the reference cycle a function to prevent necessary resolution at module import time.

Test Plan

1. Move integration test entities to different files in the first commit. Run yarn integration, ensure they fail with the following:

TypeError: Cannot read property 'getCompanionDefinition' of undefined

      634 |       inboundEdges.map(async (entityClass) => {
      635 |         return await mapMapAsync(
    > 636 |           entityClass.getCompanionDefinition().entityConfiguration.schema,
          |                       ^
      637 |           async (fieldDefinition, fieldName) => {
      638 |             const association = fieldDefinition.association;
      639 |             if (!association) {

      at ../entity/src/EntityMutator.ts:636:23
          at Array.map (<anonymous>)
      at Function.processEntityDeletionForInboundEdgesAsync (../entity/src/EntityMutator.ts:634:20)
      at DeleteMutator.deleteInternalAsync (../entity/src/EntityMutator.ts:540:25)
      at ../entity/src/EntityQueryContextProvider.ts:40:22

1. Apply fix described above. Re-run yarn integration and ensure the tests now pass.

[codecov]

Codecov Report

Merging #89 into master will increase coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #89      +/-   ##
==========================================
+ Coverage   94.61%   94.70%   +0.08%     
==========================================
  Files          60       62       +2     
  Lines        1523     1548      +25     
  Branches      167      166       -1     
==========================================
+ Hits         1441     1466      +25     
  Misses         80       80              
  Partials        2        2              

Flag	Coverage Δ
integration	94.70% <100.00%> (+0.08%)	⬆️
unittest	94.70% <100.00%> (+0.08%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
.../src/__integration-tests__/entities/ChildEntity.ts	100.00% <100.00%> (ø)
...src/__integration-tests__/entities/ParentEntity.ts	100.00% <100.00%> (ø)
packages/entity/src/EntityConfiguration.ts	100.00% <100.00%> (ø)
packages/entity/src/EntityMutator.ts	98.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86aa5b2...65b90eb. Read the comment docs.

[quinlanj]

While this solves the current dependency cycle, it doesn't solve the overall problem of cycles. The solution also adds extra complexity to inboundEdges in order to break the current cycle.

Since the entity repo is smaller than www, this might be a good place to try the prescribed solution in the article you linked, adding an index/internal.ts and have everything import from that intermediate file. This way, modules are always loaded in the same order every time.

[wschurman]

Since the entity repo is smaller than www, this might be a good place to try the prescribed solution in the article you linked, adding an index/internal.ts and have everything import from that intermediate file. This way, modules are always loaded in the same order every time.

The package entity already does this technically, but we want the library to support both use cases (repositories that both use and choose not to use index.ts files). Put another way, the usage of a library shouldn't dictate this convention for the using project as a whole. Therefore, we needed to find a compromise, and I think making this field a function is the simplest tradeoff.

Other solutions were considered, though:

• Entity "registry", where each entity is registered and this cyclical graph of reverse edges is built at runtime. The downside here is that the registry pattern isn't frequently used in JS and can also be hard to debug.
• Entity introspection, where a directory of concrete entity implementor definition filepaths is supplied to the framework initialization. The downside here is that this would dictate directory structure for a user's entities.

[ide]

This solution seems good and should hold up. There is a cyclic relationship between entity types, but it doesn't need to be resolved all at once (hence the new thunk).