validate properties like in spatie/data-transfer-object

spatie / laravel-view-models

View models in Laravel

https://spatie.be/open-source

MIT License

1.01k stars 61 forks source link

validate properties like in spatie/data-transfer-object #19

Closed Gummibeer closed 4 years ago

Gummibeer commented 4 years ago

It would be great to have a validated ViewModel like done in the spatie/data-transfer-object package. I think that requiring and extending it wouldn't work because it does not handle methods? But methods don't have to be validated because PHP will throw an exception by it's own if the return type doesn't match the type hint.

brendt commented 4 years ago

Hi Tom

I'm afraid I'm not entirely sure what you mean with this, care to elaborate?

Gummibeer commented 4 years ago

Atm it's possible to have a ViewModel with a property $author and a doc-tag which states string but fill in a user model for example. In a DataTransferObject this would result in an exception.

So what I mean is that it would be great if the ViewModel would validate itself like the DataTransferObject does to prevent any exception in a later place or unexpected behavior if I do echo $author.

brendt commented 4 years ago

Gotcha. I don't think it's worth the overhead though:

PHP 7.4 and typed properties will mitigate this issue
Right now you can use static analysis tools to warn you of these errors

I agree that it would be nice to already have this baked in, but it's not feasible to add a stronger userland type system to any given amount of our packages. The DTO package is the exception, as its main goal is to extend PHP's type system.

Are there any things I overlooked?

Gummibeer commented 4 years ago

PHP7.4 is right, static analysers will fail in multiple cases because they can't resolve variable contents filled from any non code resources. For me the ViewModel is only a DTO + render method.^^ That's why I wondered the moment I could put in everything.

brendt commented 4 years ago

Okay.If we went back in time a year, I would consider adding it. But right now, with two months before PHP 7.4, I don't think it's worth adding the overhead to the package.

Hope you can understand my point of view :)

Gummibeer commented 4 years ago

Sure - I also consider to wait for PHP7.4 to release https://github.com/Astrotomic/stancy so I don't really need it. But I'm sure that only a few will upgrade to PHP7.4 and a lot will still use the absolute minimum version that's possible. :/