Skip to content
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

Make Value Objects and DTOs easier to work with and to debug #5239

Open
2 of 4 tasks
bwaidelich opened this issue Sep 6, 2024 · 1 comment
Open
2 of 4 tasks

Make Value Objects and DTOs easier to work with and to debug #5239

bwaidelich opened this issue Sep 6, 2024 · 1 comment

Comments

@bwaidelich
Copy link
Member

bwaidelich commented Sep 6, 2024

Our Value Objects and DTOs are strict by design.
But this makes it harder to use them in non-PHP land (like Fusion or Fluid). Besides debugging stack traces gets really hard.

Suggestion

  • Implement the __debugInfo() magic method where it makes sense to improve debugging output
  • Implement the __toString() magic methods where it makes sense (specifically in value objects that represent a single simple type)
  • Create a linter rule to prevent explicit string casts in our code base ((string)$someValueObject)
  • Create a linter rule to prevent implicit string casts in our code base (e.g. foo($someValueObject) where function foo (string $bar) {})

Notes

  • @skurfuerst suggested to return the value of the VO for objects that represent a single value and some debugging string for objects that contain multiple properties (e.g. Node[aggregateId: ..., workspace: ....]) such that they are not converted by accident – for the latter we can implement __debugInfo() to provide more details
  • To prevent the implicit casting it should be enough to enforce the declare(strict_types=1); declaration in all PHP files of the core
@bwaidelich bwaidelich self-assigned this Sep 6, 2024
bwaidelich added a commit that referenced this issue Sep 6, 2024
Adds a

```
declare(strict_types=1);
```

declaration to all PHP files of the core that were missing one and implements & defines a new PHPStan rule to enforce them from now on

Related: #5239
bwaidelich added a commit that referenced this issue Sep 6, 2024
Implement the magic `__toString()` method to relevant core value objects that represent a single string value.

Note: This also adds and configures a PHPStan rule that disallows implicit casting in for those

Related: #5239
@mhsdesign
Copy link
Member

Also i think we should leverage the string casts in fusion again for node.name instead of using node.name.value as elaborated here: #4156 (comment)

The thing is the to get the site entity name we use site.name as its not a dto there. This is super confusing even for us and causes lots of bugs in the past (#5236 ...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants