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

Added support for Arrayable #84

Merged
merged 3 commits into from
Jul 24, 2024
Merged

Added support for Arrayable #84

merged 3 commits into from
Jul 24, 2024

Conversation

felixbessler
Copy link
Contributor

@felixbessler felixbessler commented Jul 23, 2024

Problem

We have a custom cast that translates a string to an object, which works fine when we use it like this:

$dto->object

But when we need the values in an array, for example for each VueJS component, we lose that value. The ->toArray() function makes it an empty array because of PHPs array casting of an object.

Solution

With this PR we are trying to add support for the Arrayable interface from Laravel. It already works like this, I just can't figure out how to test this change as easily as possible. I would need a custom cast file that returns an object, a custom object file..

And before I add all of this, I wanted to ask for your opinion on this change, and also on the testing approach. Perhaps you have a better idea?

Co-authored-by: patriziotomato <[email protected]>
@WendellAdriel
Copy link
Owner

I like the idea. 🔥
Regarding the test approach, if we want to have a good test for that, I think that you're going to need to create those custom files to make sure that this is working as expected.

@felixbessler felixbessler marked this pull request as ready for review July 24, 2024 08:10
@felixbessler
Copy link
Contributor Author

Cool, then I added a test that checks the support of Arrayable. If you want me to change anything, please let me know.

Copy link
Owner

@WendellAdriel WendellAdriel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the work on this! 💪

@WendellAdriel WendellAdriel merged commit 18e4521 into WendellAdriel:main Jul 24, 2024
8 checks passed
@felixbessler felixbessler deleted the add-support-for-arrayable branch July 24, 2024 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants