Drop support for doctrine/collections v1
#2985
Conversation
| if (! $coll instanceof BaseCollection) { | ||
| throw new LogicException(sprintf('The matching() method of the backed collection must return an instance of "%s".', BaseCollection::class)); | ||
| } |
There was a problem hiding this comment.
The default implementation of PersistableCollection extends the ArrayCollection that has a more specific return type than Selectable::matching(): ReadableCollection
https://github.com/doctrine/collections/blob/a8353e2e0cb9db12ceb87c8e76114bd885b0170f/src/ArrayCollection.php#L386
| * @param mixed $element The element to add. | ||
| * @phpstan-param T $element | ||
| * | ||
| * @return true The return value is kept for BC reasons, but will be void in doctrine/mongodb-odm 3.0. |
There was a problem hiding this comment.
I think it might make sense to do a conditional bc break (based on the version of doctrine/collections), as in https://github.com/doctrine/orm/pull/12341/changes#diff-9634c9dcbc64c9326048aaa0df50b1a5dea4214b83b56ccb5ef676bf076841f6
There was a problem hiding this comment.
Thanks for the feedback.
I would have to put all the methods of the interface in the trait in order to have compatibility with both doctrine/collections 2 and 3.
I wonder if I should extend AbstractLazyCollection instead, and just use a conditional trait for the add method.
There was a problem hiding this comment.
Maybe yes. I think PersistentCollection 49fb4e4 was introduced 3 years prior to AbstractLazyCollection (doctrine/collections#19)
Although ODM wasn't explicitly mentioned, I think it might have been hinted at in doctrine/orm#882 (comment)
There was a problem hiding this comment.
I've open an issue, I want this PR to get merged without support for doctrine/collections v3:
GromNaN
changed the title
doctrine/collections 3.0doctrine/collections 3.0
| "php": "^8.1", | ||
| "ext-mongodb": "^1.21 || ^2.0", | ||
| "doctrine/collections": "^1.5 || ^2.0", | ||
| "doctrine/collections": "^2.1", |
There was a problem hiding this comment.
doctrine/collections 1 is already not compatible with PHP 8, so this changing nothing.
| * @param mixed $element The element to add. | ||
| * @phpstan-param T $element | ||
| * | ||
| * @return true The return value is kept for BC reasons, but will be void in doctrine/mongodb-odm 3.0. |
There was a problem hiding this comment.
Thanks for the feedback.
I would have to put all the methods of the interface in the trait in order to have compatibility with both doctrine/collections 2 and 3.
I wonder if I should extend AbstractLazyCollection instead, and just use a conditional trait for the add method.
GromNaN
changed the title
doctrine/collections 3.0doctrine/collections v1
3 participants
Summary
doctrine/collections v1 can't be installed already because of the PHP version compatibility.
doctrine/collections3.0 adds return types, this PR ensure we have the correct types documented.Collection::add()returnsvoid, we currently returntrue. BC break will be done in mongodb-odm 3.0ArrayCollection::matching(): Collectionmethod is more restrictive thanSelectable::matching(): ReadableCollection.