SOF3 / await-generator

Write code in async/await style in PHP using generators.
https://sof3.github.io/await-generator/master/
Apache License 2.0
123 stars 15 forks source link

Added Await::promise() and deprecated constant yields #105

Closed SOF3 closed 2 years ago

SOF3 commented 2 years ago

The yield Await::ONCE syntax causes constant confusion to new users.It would be simpler to use an API similar to JavaScript Promise, which simply creates a promise by passing a resolve/reject closure.

Note that Await::promise differs from JavaScript Promise in that $closure is NOT executed until it is yielded and processed by an Await runtime. This behaviour is intentional, because the await-generator philosophy is that a generator must either be Await::g2ced (to execute in background) or yielded (or yield from) so that it starts executor. A generator that has not been yielded should not do anything, This is because no side effects would be expected before the yield/g2c starts since the suspension points are transparent to the caller.

ColinHDev commented 2 years ago

With deprecating those constants, do you mean entirely removing or still using them internally?

SOF3 commented 2 years ago

Still using them internally. Not planning to break BC. However, there may be a way to identify whether they are created internally or by users, then generate a deprecatioin warning.

ColinHDev commented 2 years ago

You could add the @deprecated tag to them for now and once the project can receive the bc break, their visibility could be changed to private or protected.

ColinHDev commented 2 years ago

Also, it could be useful to add a @param / @phpstan-param tag for the closure, so that users can easily see which signature is required by that method.

This would probably look something like this...

/**
 * @phpstan-param Closure(Closure(mixed=): void, Closure(Throwable): void): void $closure
 */

Might be a bit crowded

SOF3 commented 2 years ago

You could add the @deprecated tag to them for now and once the project can receive the bc break, their visibility could be changed to private or protected.

Yes, I haven't decided whether to deprecate them at this point.

SOF3 commented 2 years ago

Also, it could be useful to add a @param / @phpstan-param tag for the closure, so that users can easily see which signature is required by that method.

This would probably look something like this...

/**
 * @phpstan-param Closure(Closure(mixed=): void, Closure(Throwable): void): void $closure
 */

Might be a bit crowded

Oops, I forgot.

ColinHDev commented 2 years ago

You could add the @deprecated tag to them for now and once the project can receive the bc break, their visibility could be changed to private or protected.

Yes, I haven't decided whether to deprecate them at this point.

Someone might not want to do

$somedata = yield Await::promise(
            function(\Closure $onSuccess, \Closure $onError) use ($someID) : void {
                $this->getSomeData($someID, $onSuccess, $onError);
            }
        );
ColinHDev commented 2 years ago

Oops, I forgot.

Do you want to annotate the return type? Probably only PHPStan will care. Should be something like:

/**
 * @phpstan-return Generator<int, string, Closure(mixed=): void|Closure(Throwable): void, mixed>
 */
SOF3 commented 2 years ago

You could add the @deprecated tag to them for now and once the project can receive the bc break, their visibility could be changed to private or protected.

Yes, I haven't decided whether to deprecate them at this point.

Someone might not want to do

$somedata = yield Await::promise(
            function(\Closure $onSuccess, \Closure $onError) use ($someID) : void {
                $this->getSomeData($someID, $onSuccess, $onError);
            }
        );

I mean you could totally write it with arrow functions

yield Await::promise(fn($resolve, $reject) => $this->getSomeData($someID, $resolve, $reject));
SOF3 commented 2 years ago

Oops, I forgot.

Do you want to annotate the return type? Probably only PHPStan will care. Should be something like:

/**
 * @phpstan-return Generator<int, string, Closure(mixed=): void|Closure(Throwable): void, mixed>
 */

One of the great things about this RFC is that we can simplify the type signature outside of await-generator. For example, people just need to write

/**
 * @phpstan-return Generator<Await, Await, Await, T>
 */

While there isn't actually an Await instance going around, it is transparent outside of await-generator, so people can declare types much more easily.

ColinHDev commented 2 years ago

You could add the @deprecated tag to them for now and once the project can receive the bc break, their visibility could be changed to private or protected.

Yes, I haven't decided whether to deprecate them at this point.

Someone might not want to do

$somedata = yield Await::promise(
            function(\Closure $onSuccess, \Closure $onError) use ($someID) : void {
                $this->getSomeData($someID, $onSuccess, $onError);
            }
        );

I mean you could totally write it with arrow functions

yield Await::promise(fn($resolve, $reject) => $this->getSomeData($someID, $resolve, $reject));

Oh, you are right. Sorry, always forgetting about those..

codecov[bot] commented 2 years ago

Codecov Report

Merging #105 (8b775af) into master (12e217f) will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #105   +/-   ##
=========================================
  Coverage     99.63%   99.63%           
- Complexity       98       99    +1     
=========================================
  Files             8        8           
  Lines           271      276    +5     
=========================================
+ Hits            270      275    +5     
  Misses            1        1           
Impacted Files Coverage Δ
await-generator/src/SOFe/AwaitGenerator/Await.php 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 12e217f...8b775af. Read the comment docs.