-
-
Notifications
You must be signed in to change notification settings - Fork 14
feat: Add #[CollectedBy] attribute
#331
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
base: main
Are you sure you want to change the base?
feat: Add #[CollectedBy] attribute
#331
Conversation
Ports Laravel's #[CollectedBy] attribute to Hypervel, allowing models to declaratively specify a custom collection class. - Add CollectedBy attribute class - Add HasCollection trait with newCollection() and attribute resolution - Modify Model to use HasCollection trait - Add tests (9 tests, 16 assertions)
| if (! isset($attributes[0])) { | ||
| return null; | ||
| } | ||
|
|
||
| // @phpstan-ignore return.type (attribute stores generic Model type, but we know it's compatible with static) | ||
| return $attributes[0]->newInstance()->collectionClass; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @binaryfire , just curious about the code difference between Laravel here: https://github.com/laravel/framework/blob/74cc0aaf411146af4e616ce6671811998568c302/src/Illuminate/Database/Eloquent/HasCollection.php#L50C9-L54C50
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @albertcht. The difference is intentional. Laravel's approach silently falls back to the default collection if someone writes #[CollectedBy] without an argument (the ! isset($attributes[0]->getArguments()[0]) guard). But that's a bug in user code - the attribute has a required parameter.
Using newInstance()->collectionClass means PHP throws ArgumentCountError if the attribute is malformed, instead of silently falling back. That feels like the right approach here - the developer sees their mistake and can fix it.
| */ | ||
| public function newCollection(array $models = []): Collection | ||
| { | ||
| static::$resolvedCollectionClasses[static::class] ??= ($this->resolveCollectionFromAttribute() ?? Collection::class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @binaryfire , what do you think if we support customizing collection type via static::$collectionClass property as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@albertcht Done
…attribute # Conflicts: # src/core/src/Database/Eloquent/Model.php
Adds a static $collectionClass property to Model as an alternative to the #[CollectedBy] attribute for specifying custom collection classes. - Add protected static $collectionClass property to Model with default Collection::class - Update HasCollection trait to fall back to static::$collectionClass - Fallback chain: #[CollectedBy] attribute → $collectionClass property - Add tests for property-based customization and attribute precedence
Extends collection customization support to pivot models, matching Laravel's behavior where Pivot inherits collection functionality from Model. Changes: - Add HasCollection trait and $collectionClass property to Pivot - Add HasCollection trait and $collectionClass property to MorphPivot - Loosen Collection's TModel constraint from Hypervel\Model to Hyperf\Model (allows Collection to work with any Hyperf-based model including pivots) - Simplify generic annotations in HasCollection for broader compatibility - Add PivotCollectionTest with 9 tests covering both Pivot and MorphPivot Pivot models can now use #[CollectedBy] attribute or override $collectionClass to specify custom collection classes, just like regular models.
|
@albertcht I've also added Since Hypervel's pivot classes extend Hyperf's base classes (not Hypervel's Model), they don't inherit collection functionality automatically. I've added the HasCollection trait and This required changing |
This PR ports Laravel's
#[CollectedBy]attribute to Hypervel, allowing models to declaratively specify a custom collection class.Summary
#[CollectedBy]attribute for declaring custom collection classes on modelsHasCollectiontrait providing thenewCollection()method with attribute resolutionModelto use the newHasCollectiontraitUsage
Changes
New Files
src/core/src/Database/Eloquent/Attributes/CollectedBy.php- The attribute classsrc/core/src/Database/Eloquent/Concerns/HasCollection.php- Trait providingnewCollection()with attribute resolutiontests/Core/Database/Eloquent/Concerns/HasCollectionTest.php- Tests (9 tests, 16 assertions)Modified Files
src/core/src/Database/Eloquent/Model.php- AddedHasCollectiontrait, removed inlinenewCollection()methodHow It Works
newCollection()is called on a model, theHasCollectiontrait checks for a#[CollectedBy]attributeHypervel\Database\Eloquent\CollectionNotes
#[CollectedBy]attribute if they want a custom collection