Skip to content

Conversation

@Jade-GG
Copy link
Collaborator

@Jade-GG Jade-GG commented Dec 17, 2025

I ran into the issue that images did not work anymore. This ended up being because the $value given to attribute casts was always null.

By hooking into the getAttributeFromArray function we can send the right value through. Alternatively, we could hook into getAttributes() and merge everything into there, however that would add a significant performance overhead.

Comment on lines +207 to +210
protected function getAttributeFromArray($key)
{
return parent::getAttributeFromArray($key) ?? $this->getCustomAttribute($key);
}
Copy link
Member

Choose a reason for hiding this comment

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

Is this to patch the internal function for $value to get the correct value?
What about calling $this->getAttributeFromArray('price') ?? $this->getCustomAttribute('price') manually?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, you definitely could do that manually each time. It would turn the functions for images from

protected function image(): Attribute
{
    return Attribute::get($this->getImageFrom(...));
}

into something like:

protected function image(): Attribute
{
    return Attribute::get(function() {
        $value = $this->getCustomAttribute('image')?->value;
        $this->getImageFrom($value);
    });
}

I'm definitely not a fan of that.

You can see how I've also removed the need for this from the price attribute in this PR. Personally I don't think it should be necessary to call such an internal function every time, especially because there's no guarantee that it'll stay the same in the future.

@royduin
Copy link
Member

royduin commented Jan 6, 2026

This doesn't have any impact on performance?

@royduin
Copy link
Member

royduin commented Jan 13, 2026

@Jade-GG + @indykoning 😇

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.

4 participants