Skip to content

protected $previous = [] in base Model breaks Eloquent relationships named previous when accessed from within the Model #55828

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

Open
MannikJ opened this issue May 22, 2025 · 4 comments

Comments

@MannikJ
Copy link
Contributor

MannikJ commented May 22, 2025

Laravel Version

12.15.0

PHP Version

8.4.7

Database Driver & Version

MariaDB 11.7.2

Description

Version affected: Laravel v12.15.0
Introduced by: #55729

After upgrading from Laravel 12.14.1 to 12.15.0, existing applications using an Eloquent relationship named previous are broken due to a new class property:

protected $previous = [];

This property was introduced in the Model class to "preserve" a relationship internally, but it shadows any existing Eloquent relationship using the name previous.


Impact

In PHP, class properties take precedence over __get() dynamic access, so:

$this->previous

now always returns the protected $previous array, not the related model.

This causes existing apps with a relationship like:

public function previous()
{
    return $this->belongsTo(Task::class, 'previous_id');
}

to suddenly break, returning [] instead of a Task instance.


Why This is a Breaking Change

  • Laravel's own conventions encourage expressive, semantic relation names like previous, next, parent, etc.
  • This change silently breaks existing public API contracts with no warning or deprecation notice.
  • It introduces an invisible namespace collision between class properties and relations.
  • It violates the principle of least surprise in a way that’s hard to debug (e.g., returns an empty array, not null or exception).

Suggested Fix

At the very least there has to be warning about this problem. In my opinion it should never have been released as a minor update. I would vote for to actually revert and postpone this to a major version.

This incident highlights a larger structural limitation in Laravel's current Eloquent model system: attributes and class properties share the same namespace, and magic access provides no guardrails against collisions.

A longer-term fix could involve refactoring the HasAttributes trait to encapsulate model attributes in a dedicated container, clearly separating user-defined class properties from Eloquent-managed state. This would eliminate accidental shadowing and make models safer and more predictable at scale.


References

Please consider reverting this change or implementing a safer alternative.


Steps To Reproduce

  1. Create a model with a previous() relationship:

    class Task extends Model
    {
        public function previous()
        {
            return $this->belongsTo(Task::class, 'previous_id');
        }
    }
    
  2. Load and access the relation:

    $task = Task::with('previous')->find(1);
    dd($task->previous); // returns []
    
  3. If you call the relation manually, it works:

    dd($task->getRelation('previous')); // correct Task model
    
  4. Downgrade to Laravel 12.14.1 → works again as expected.

@crynobone
Copy link
Member

crynobone commented May 22, 2025

Hey there, thanks for reporting this issue.

We'll need more info and/or code to debug this further. Can you please create a repository with the command below, commit the code that reproduces the issue as one separate commit on the main/master branch and share the repository here?

Please make sure that you have the latest version of the Laravel installer in order to run this command. Please also make sure you have both Git & the GitHub CLI tool properly set up.

laravel new bug-report --github="--public"

Do not amend and create a separate commit with your custom changes. After you've posted the repository, we'll try to reproduce the issue.

Thanks!


In PHP, class properties take precedence over __get() dynamic access

This is only true for public but not protected. Otherwise, PHP is not a secure language.

Unless you're doing something funky with Eloquent. a new protected property doesn't have any effect. But, we can confirm this we a reproduction repository.

@MannikJ
Copy link
Contributor Author

MannikJ commented May 23, 2025

Thanks for the quick reply!

Here’s a minimal reproduction that shows the issue clearly:
https://github.com/MannikJ/laravel-shadowing-bug

Just to clarify: Even protected class properties will shadow __get() when accessed from within the class. That’s exactly what the test demonstrates — the previous relationship works when accessed externally, but is silently shadowed internally by the protected $previous = [] property introduced in Laravel 12.15.0.

This behavior worked in 12.14.x and is broken in 12.15.0. It's not just a naming preference — it’s a silent behavioral change with real runtime consequences for apps relying on previous() relationships.

I don't think accessing relationships via magic accessor is funky at all.

@MannikJ MannikJ changed the title protected $previous = [] in base Model breaks Eloquent relationships named previous protected $previous = [] in base Model breaks Eloquent relationships named previous when accessed from within the Model via $this->previous May 23, 2025
@MannikJ MannikJ changed the title protected $previous = [] in base Model breaks Eloquent relationships named previous when accessed from within the Model via $this->previous protected $previous = [] in base Model breaks Eloquent relationships named previous when accessed from within the Model May 23, 2025
@crynobone
Copy link
Member

I honestly would say it shouldn't be hard to handle $this->previous to $this->getRelation('previous'); just inside the model. It a different scenario when $model->previous (outside the class scope) acts that way. But will still bring this up the the team.

@MannikJ
Copy link
Contributor Author

MannikJ commented May 23, 2025

I have already fixed it for us, by using $this->getAttribute('previous'), because this is what handles non-existent relations in the same way as the magic getter would do.

The maint point is still, that this is a potential breaking change, which was quite hard to debug, so it should not have found its way into a minor update.
At the very least there should be some warning about this.

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