New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect cyclic dependencies in property definitions #1799

Open
bajtos opened this Issue Oct 4, 2018 · 1 comment

Comments

Projects
None yet
1 participant
@bajtos
Copy link
Member

bajtos commented Oct 4, 2018

While working on belongsTo relation in #1618, we discovered a need to detect circular dependencies in property definitions and provide a helpful error to users.

Here is a PoC code that did not make it to master yet:

/**
* Decorator for model properties
* @param definition
* @returns {(target:any, key:string)}
*/
export function property(definition?: Partial<PropertyDefinition>) {
const isCyclic =
definition &&
(definition as Object).hasOwnProperty('type') &&
!definition.type;
const isCyclicArray =
definition &&
(definition.type === Array || definition.type === 'array') &&
(definition as object).hasOwnProperty('itemType') &&
!definition.itemType;
if (isCyclic || isCyclicArray) {
// this path is taken when cyclic dependency is detected
// in that case, a TypeResolver should be used instead
throw new Error(ERR_TARGET_UNDEFINED);
}

We should take a closer look at different ways how a cyclic relation can be introduced, write tests to reproduce those cases and then fix the implementation to throw an error that explains the user what went wrong and how to fix the problem.

Acceptance criteria

TBD

@bajtos

This comment has been minimized.

Copy link
Member

bajtos commented Oct 4, 2018

Few examples of different ways how to create a cyclic dependency. Some of them are rejected by the compiler if all code is in the single file (but may not be detected if each model has its own source code file).

Property types read from design:type metadata:

class Wife {
 @property()
 husband: Husband;
}

class Husband {
  @property()
  wife: Wife;
}

Property types specified explicitly via a model class:

class Wife {
 @property({type: Husband})
 husband: Husband;
}

class Husband {
  @property({type: Wife})
  wife: Wife;
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment