Minor BC breaks #2612

gskema · 2025-01-14T13:32:15Z

gskema
Jan 14, 2025

In version 6.0.2 RedisCluster method signature was:

public function hscan(string $key, ?int &$iterator, ?string $pattern = null, int $count = 0): array|bool;

in 6.1.0 it was changed to

public function hscan(string $key, null|int|string &$iterator, ?string $pattern = null, int $count = 0): array|bool;

Now this is a small problem when I am extending the class to add wrapper methods

// created on version 6.0.2
class TrackedRedisCluster extends RedisCluster {
   public function hscan(string $key, ?int &$iterator, ?string $pattern = null, int $count = 0): array|bool {
      // some performance tracking code, etc.
   }
}

When the extension is uptrade to >=6.1.0 this error will pop up:

Compile Error: Declaration of
 RedisCluster6Proxy::hscan(string $key, ?int &$iterator, ?string $pattern = null, int $count = 0): array|bool
 must be compatible with
 RedisCluster::hscan(string $key, string|int|null &$iterator, ?string $pattern = null, int $count = 0): array|bool

There is a way to bypass it though by widening parameter types in child class to "everything" (mixed):

   public function hscan($key, &$iterator, $pattern = null, $count = 0) {}

This is essentially what Symfony also does (removes types).
So I just wanted to point this out that this is kind of a breaking change. I checked the CHANGELOG and
the signature changes are not mentioned there. Does this warrant a whole new 7.0 version? Maybe not, but it could be at least [MINOR BC BREAK] in the changelog - other repos do it.

michael-grunder · 2025-01-14T17:19:50Z

michael-grunder
Jan 14, 2025
Maintainer

Yes, the breaking change is my fault. I didn't consider that so many people actually subclass PhpRedis. Too much time doing C I guess.

There's also a discussion around this in #2562 and #2564.

Does this warrant a whole new 7.0 version? Maybe not, but it could be at least [MINOR BC BREAK] in the changelog

I'm not sure. We should document the issues somewhere prominent.

Ideally I'd like to break a few different things in 7.0. For now at least most of the frameworks have been able to work around these AFAIK.

0 replies

gskema · 2025-01-15T07:34:27Z

gskema
Jan 15, 2025
Author

Thank you for the clarification. Just wanted to ping about this issue, but I see there have been a couple of issues raised already and the team is aware. So I will close this.

0 replies

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Minor BC breaks #2612

Uh oh!

{{title}}

Uh oh!

Replies: 2 comments

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Select a reply

Uh oh!

Uh oh!

Minor BC breaks #2612

Uh oh!

gskema Jan 14, 2025

Replies: 2 comments

Uh oh!

michael-grunder Jan 14, 2025 Maintainer

Uh oh!

gskema Jan 15, 2025 Author

gskema
Jan 14, 2025

michael-grunder
Jan 14, 2025
Maintainer

gskema
Jan 15, 2025
Author