4lessandrodev / rich-domain

A lib to help you create a robust project based on domain driven-design (ddd) principles with typescript and zero dependencies.
https://www.npmjs.com/package/rich-domain
MIT License
122 stars 5 forks source link

Incorrect Return Type Inferred for Value Objects with Singular and Multiple Attributes #148

Closed 4lessandrodev closed 5 months ago

4lessandrodev commented 6 months ago

Describe the bug

When a value object has only one attribute, the current behavior leads to an incorrect inference of the return type. Instead of returning an object with the attribute value directly, the return type is inferred as undefined. Conversely, when the value object has multiple attributes, the return type correctly infers an object with key-value pairs representing the attributes.

To Reproduce

  1. Define a value object class inheriting from ValueObject.
  2. Ensure the value object class has a constructor accepting an interface defining its attributes.
  3. Implement a factory method within the value object class to create instances.
  4. Create instances of the value object class with singular and multiple attributes.
  5. Retrieve the object representation using the toObject method.

Expected behavior

When the value object has a singular attribute, the toObject method should directly return the attribute value. When the value object has multiple attributes, the toObject method should return an object with key-value pairs representing the attributes.

Actual Behavior:

Currently, when the value object has a singular attribute, the toObject method incorrectly infers the return type as undefined. When the value object has multiple attributes, the toObject method correctly returns an object with key-value pairs.

Screenshots

image


// Example demonstrating the issue
const vo = Street.create('10 Avenue').value();
const street = vo.toObject();
street.value // returns undefined, because vo has a single key

When multiple keys

image


// Example demonstrating correct behavior
const vo = Address.create({ city: 'Rio de Janeiro', street: 'Av Copacabana' }).value();
const payload = vo.toObject();
payload.city // returns 'Rio de Janeiro'
payload.street // returns 'Av Copacabana'

Lib Version rich-domain v1.22.1

Proposed Solution:

Adjust the implementation of the toObject method in the ValueObject class to correctly infer the return type based on the number of attributes present in the value object. If the value object has only one attribute, return the attribute value directly; if it has multiple attributes, return an object with key-value pairs representing the attributes.

4lessandrodev commented 6 months ago

Test Case: https://github.com/4lessandrodev/rich-domain/commit/0a3d7ca907ff23c978e59d057c640d980983fb4e

4lessandrodev commented 6 months ago

Reference: #145

hikinine commented 6 months ago

I mean,

type Props = {
  value: string
}
class Street extends ValueObject<Props> {}

it is not a primitive value, its an object with a key named "value" which has a primitive value image

const street = new Street({ value: 'RUA A' })
const obj = street.toObject()

Result will be an object like:

{ value: 'RUA A' }

As supposed to be, am I wrong?

As I understand, an primitive value would be like

class Street extends ValueObject<string> {}
const street = new Street('RUA A' )
const obj = street.toObject()

Result will be like:

   // string
hikinine commented 6 months ago

Ok, i get it now. Auto mapper implementation removes single key objects and transform it to primitive (is it the expected behavior?). Ill study a little bit more the method

hikinine commented 6 months ago

I think it's a design decision (looks like both are acceptable) I didn't realized that auto mapper implementation removes object with single key.

Maybe that is the expected behavior, if so, we'll need to change the AutoMapperSerialize to support it.

BUT

If we have something like:

type PersonProps= {
   fullName: string
}
export class Person extends ValueObject<PersonProps> {}

const person = new Person({ fullName: 'John Doe' });
const result = person.toObject();

Result will be type string instead of { fullName: string }.

let me know your thoughts.

Solution for stopping transform single key objects into primitives

image

4lessandrodev commented 6 months ago

As I mentioned in issue #145, the standard for a value object when it has only a single attribute is to shorten the access path to the attribute from the entity.

In the example you mentioned, the user would be an entity, and its attribute would be a value object.

Example:

const user = {
   fullName: "Joao"
}

Instead of

const user = {
   fullName: {
       value: "Joao"
   }
}

Thus, by default, when there is a single attribute, this path is shortened to facilitate access from the entity.

On the other hand, the natural behavior when there are more than one attribute is understood to be a structured object, and therefore this abbreviation is not adopted.

About Primitive Values

You mentioned adopting the value object as a primitive:


class Street extends ValueObject<string> {}

While acceptable and usable, there are complications such as the instance's getters and setters. They are structured to work with Props in an object; the props pattern is an object.


const vo = new Street("sample");

vo.set("");
vo.get("");
// This and some other methods do not work with instances with props of primitive types, 
// so it is recommended that props always be an object.
4lessandrodev commented 6 months ago

I tested some possibilities of conditional types and realized that it's not good to perform this type of type checking considering the quantity of properties of an object.

So, for the sake of typing, I see that the best decision is:


class Street extends ValueObject<string> {}