Skip to content
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

fix: return type should be iterable #5443

Closed
wants to merge 1 commit into from

Conversation

develth
Copy link
Contributor

@develth develth commented Mar 8, 2023

... to match for example CollectionProvideres return type on pagination

Q A
Branch? main
Tickets
License MIT
Doc PR

... to match for example CollectionProvideres return type on pagination
@@ -29,5 +29,5 @@ interface ProviderInterface
*
* @return T|Pagination\PartialPaginatorInterface<T>|iterable<T>|null
*/
public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null;
public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|iterable|null;
Copy link
Member

Choose a reason for hiding this comment

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

I think that this is breaking compatibility

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But regarding this it's not compatible

public function provide(Operation $operation, array $uriVariables = [], array $context = []): iterable

@soyuka
Copy link
Member

soyuka commented Mar 9, 2023

we found our reasoning: #5024

@soyuka soyuka closed this Mar 9, 2023
@develth
Copy link
Contributor Author

develth commented Mar 9, 2023

@soyuka

FYI object|array is identical to object|iterable even in PHP 8.0 and 8.1. :)

Ok, thought that on the otherview object|iterable is not identical to object|array, because Iteratable also contains Traversable which would not match with array.

Thx for information.

@soyuka
Copy link
Member

soyuka commented Mar 9, 2023

Mhh actually it looks like a PHP issue php/php-src@bb79ef7

Iterable is a built-in compile time type alias for array|Traversable. From its introduction in PHP 7.1.0 and prior to PHP 8.2.0, iterable was a built-in pseudo-type that acted as the aforementioned type alias and can be used as a type declaration. An iterable type can be used in foreach and with yield from within a generator.

reopening this

@soyuka soyuka reopened this Mar 9, 2023
@soyuka
Copy link
Member

soyuka commented Mar 10, 2023

We need to find out what PHP's choice is for 8.3 if we should revert or not . but basically iterable and array are compatible so it doesn't change much

@stale
Copy link

stale bot commented May 9, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 9, 2023
@nesl247
Copy link
Contributor

nesl247 commented May 9, 2023

This is still an issue.

@stale stale bot removed the stale label May 9, 2023
@soyuka
Copy link
Member

soyuka commented May 22, 2023

why @nesl247 ?

@nesl247
Copy link
Contributor

nesl247 commented May 30, 2023

@soyuka I didn't see any changes made for this. Were there?

@soyuka
Copy link
Member

soyuka commented May 30, 2023

why is this a problem?

@nesl247
Copy link
Contributor

nesl247 commented May 30, 2023

why is this a problem?

#5303

@soyuka
Copy link
Member

soyuka commented May 30, 2023

I don't get the issue as array and iterables should be compatible. Also not sure what PHP's decision is for 8.3. I'm afraid that the fix is wrong regarding the types, when using ProviderInterface<array> you'd get iterable<array> which isn't wanted no?

@stale
Copy link

stale bot commented Jul 29, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 29, 2023
@stale stale bot closed this Aug 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants