Skip to content

[11.x] Fallback to parent methods on HasUniqueStringIds trait#54096

Merged
taylorotwell merged 1 commit into
laravel:11.xfrom
hafezdivandari:11.x-unique-id-fallback
Jan 6, 2025
Merged

[11.x] Fallback to parent methods on HasUniqueStringIds trait#54096
taylorotwell merged 1 commit into
laravel:11.xfrom
hafezdivandari:11.x-unique-id-fallback

Conversation

@hafezdivandari

Copy link
Copy Markdown
Contributor

This PR makes HasUniqueStringIds trait respect $usesUniqueIds property and fallback to parent methods:

  • Determining if the model uses unique IDs before overriding uniqueIds() method and fallback to parent if not.
  • Fallback to parent method on getKeyType() and getIncrementing() method for consistency.

This is useful when a model is using this trait optionally by setting $usesUniqueIds property.

@taylorotwell taylorotwell merged commit 6755311 into laravel:11.x Jan 6, 2025
@hafezdivandari hafezdivandari deleted the 11.x-unique-id-fallback branch January 6, 2025 20:10
public function uniqueIds()
{
return [$this->getKeyName()];
return $this->usesUniqueIds() ? [$this->getKeyName()] : parent::uniqueIds();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@taylorotwell I don't really understand why, but this change specifically breaks PHPStan/Larastan.

It causes static analysis to think that every model ->id property is int.

I don't know why $this->usesUniqueIds() is false when running static analysis, though.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is happening to us as well, the translation plugin by Spatie is getting int and then this mess up all of the IDs returning so as a quick fix we had to go back to v11.37.0.

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