composer.lock

Package changes

Dev Package changes

Settings · Docs · Powered by Private Packagist

The composer.lock diff comment has been updated to reflect new changes in this PR.

The composer.lock diff comment has been updated to reflect new changes in this PR.

The composer.lock diff comment has been updated to reflect new changes in this PR.

Needs a react/promise 3.x release to happen first, then this can be finalized

This has now (finally) happened: https://github.com/reactphp/promise/releases/tag/v3.0.0

If there is a way I can help move this forward, please let me know.

Yup I've seen that, thanks! It's on my list for next week

The composer.lock diff comment has been updated to reflect new changes in this PR.

Some feedback @WyriHaximus @clue:

• Overall this looks great IMO, thanks!
• resolve() requires an arg which is fine by me, but that means when you use it you always end up with at least a PromiseInterface<null>. However if you do ->then(function() { }) you get a PromiseInterface. So IMO the latter maybe should be forbidden, or I'm not sure how to best type this if I expect nullI kinda don't care if it's void obviously as a consumer as really this is never going to be consumed either way. But if I type it asvoidthen I cannot useresolve()anymore. For now I typedPromiseInterface<void|null>` but it's a bit ugly.
• It seems like resolve(true); yields a PromiseInterface<bool> and that is not compatible with a PromiseInterface<true> which makes sense of course but seems like a bug, probably that's a PHPStan generics bug though.

I still need to fix a couple minor problems in FileDownloader with a fresher head, but otherwise I think this is mergeable.

This one has been something we've discussed a lot and it breaks down to this: Effectively a void method translates to a null return so the promise PromiseInterface<null>. We're literally giving you back when PHP gives us: https://3v4l.org/PvkVI It took a bit for me to make sense as well tbh.

That is what I would expect and IMO it makes sense as there is no real void type in PHP, but it doesn't seem to be what happens:

        \PHPStan\dumpType(\React\Promise\resolve(null));
        $promise = new Promise(function ($resolver, $canceler): void { $resolver(null); });
        \PHPStan\dumpType($promise->then(function(): void {}));

  231    Dumped type: React\Promise\PromiseInterface<null>
  233    Dumped type: React\Promise\PromiseInterface<void>

Hopefully you see what I mean now. To always satisfy null one would have to explicitly return null. It's probably fine, I guess I should fix it that way because it is more semantically correct.

Ok I kept the type as PromiseInterface<void|null> for now, because it was just too ugly to add return null everywhere for no good reason..

IMO it'd be nice if there was a phpstan extension perhaps that would convert PromiseInterface<void> to PromiseInterface<void> because they are really equivalent from an API consumer perspective. But this void indicates that the return value should not be used/consumed, so perhaps the better fix would be to allow empty resolve() again, for when there is no value to resolve with, and that should then yield a PromiseInterface<void> in the PHPStan extension even tho the default value probably would have to be null.

Some feedback @WyriHaximus @clue:

• Overall this looks great IMO, thanks!

@Seldaek Thanks for the feedback, glad you like the release!

• resolve() requires an arg which is fine by me, but that means when you use it you always end up with at least a PromiseInterface<null>. However if you do ->then(function() { }) you get a PromiseInterface. So IMO the latter maybe should be forbidden, or I'm not sure how to best type this if I expect nullI kinda don't care if it's void obviously as a consumer as really this is never going to be consumed either way. But if I type it as void then I cannot use resolve() anymore. For now I typedPromiseInterface<void|null> but it's a bit ugly.

Excellent question! Probably deserves a longer-form blog post, I'll put this on my list :)

In a gist, there is in fact a subtle, but noticeable difference between PromiseInterface<void> and PromiseInterface<null>, not on a functional but a semantic level. This is probably best explained when looking at "normal" return types where a void means "returns nothing at all and should not be used" and null means "returns nothing in place of T" and is usually used like ?T or T|null. This also implies you only use void as a single return type (i.e. void|int is invalid) and null as part of a union type (i.e. null|int is valid). Accordingly, null|void would be an invalid type.

Very similar to your above examples, this is also covered by our test suite:

assertType('React\Promise\PromiseInterface<null>', resolve(null));
assertType('React\Promise\PromiseInterface<void>', resolve(null)->then(function (null $value): void { }));
assertType('React\Promise\PromiseInterface<null>', resolve(null)->then(function (null $value): void { })->then(function (null $value) { }));

(See https://github.com/reactphp/promise/blob/c86753c76fd3be465d93b308f18d189f01a22be4/tests/types/resolve.php#L44C3-L44C3)

Semantically, the above holds true and is what should be relied upon. On a functional level, you're indeed right that a PromiseInterface<void> does in fact pass a null value to the fulfillment handler simply because there is no such thing as a void value. Accordingly, a PromiseInterface<null|void> would be an invalid type and should probably be replaced with a PromiseInterface<void>.

Hope this helps to clear things up a bit, but definitely warrants a longer post in the future, sponsors welcome I guess! 😉

Semantically, the above holds true and is what should be relied upon. On a functional level, you're indeed right that a PromiseInterface<void> does in fact pass a null value to the fulfillment handler simply because there is no such thing as a void value. Accordingly, a PromiseInterface<null|void> would be an invalid type and should probably be replaced with a PromiseInterface<void>.

@clue thanks but for me it's not quite clear why null|void is invalid. As it is a return type both are valid return types, but one single function cannot return both of course.

But as resolve() does not allow you to produce voids, it means if you declare a void type you ban the use of resolve(). That's my main gripe I guess.

Im going to tag this here #11563 as it might be related and broken by this (purely speculation, as it contains the word promise in the error and is a recent change :) )

@PhilETaylor it is related because of the end of chain detection added in v3, let me respond on that issue a bit more in-depth on your issue.

Absolutely no rush - and Im not looking for support at all :) but thought best to report it early.

Absolutely no rush - and Im not looking for support at all :) but thought best to report it early.

Really appreciate that, it a bit BC break for our consumers of that package. But I'm excited that we already get to see it pay off this quickly 👍.

Semantically, the above holds true and is what should be relied upon. On a functional level, you're indeed right that a PromiseInterface<void> does in fact pass a null value to the fulfillment handler simply because there is no such thing as a void value. Accordingly, a PromiseInterface<null|void> would be an invalid type and should probably be replaced with a PromiseInterface<void>.

@clue thanks but for me it's not quite clear why null|void is invalid. As it is a return type both are valid return types, but one single function cannot return both of course.

@Seldaek Late reply, but my main point is that promise template types would always be the async equivalent of a synchronous return value, i.e. PromiseInterface<int> would mimic an async int return. A PHP function may return union types as of PHP 8+, but null|void and void|null and ?void are explicitly forbidden in PHP, see https://3v4l.org/diW7r and https://3v4l.org/U4a3K. Accordingly, I would argue that a PromiseInterface<null|void> would be invalid and should most likely be replaced with a PromiseInterface<void> instead.

But as resolve() does not allow you to produce voids, it means if you declare a void type you ban the use of resolve(). That's my main gripe I guess.

I can see where you're coming from and agree that it's inconsistent there's no easy way to generate a PromiseInterface<void> without using promise chains. I'm not sure how useful this is in practice, but happy to continue this discussion in reactphp/promise if you feel like opening a ticket perhaps? 👍

Yeah I guess not worth adding a resolveVoid() which would kinda be the only way to separate creating a PromiseInterface<void> from a null one with resolve(). So let's bury this, I'll live with it :)