-
Notifications
You must be signed in to change notification settings - Fork 660
Conversation
Codecov Report
@@ Coverage Diff @@
## 2.x #662 +/- ##
============================================
- Coverage 90.71% 90.61% -0.10%
Complexity 2 2
============================================
Files 60 60
Lines 1690 1694 +4
============================================
+ Hits 1533 1535 +2
- Misses 157 159 +2
Continue to review full report at Codecov.
|
@rennokki any plans to merge ? |
It's a bit tricky because you can already call the |
i see - currently #661 is in the way for the |
Actually it throws even anothe error when trying to bind to Class which extends Illuminate\Contracts\Container\BindingResolutionException
Target [BeyondCode\LaravelWebSockets\Contracts\ChannelManager] is not instantiable while building [App\API\SelectRandomUserFromChannel].
at vendor/laravel/framework/src/Illuminate/Container/Container.php:1038
1034▕ } else {
1035▕ $message = "Target [$concrete] is not instantiable.";
1036▕ }
1037▕
➜ 1038▕ throw new BindingResolutionException($message);
1039▕ }
1040▕
1041▕ /**
1042▕ * Throw an exception for an unresolvable primitive.
+17 vendor frames
18 app/Providers/AppServiceProvider.php:31
Illuminate\Support\Facades\Facade::__callStatic("get")
+7 vendor frames
26 [internal]:0
Illuminate\Foundation\Application::Illuminate\Foundation\{closure}(Object(App\Providers\AppServiceProvider)) |
It's weird because the issue #661 that you sent to me has a different error message than the one you specified now. The error message that you specified now it's because the ChannelManager class is not instantiable. The sockets cannot be accessed outside the server, only within the files that are strictly used by the start command Show me a sample of your code using |
Therefor i think my pull request is the only way to hook into the correct server context - for example this class <?php
namespace App\API;
use BeyondCode\LaravelWebSockets\API\Controller;
use Illuminate\Http\Request;
use Illuminate\Http\Response;
use React\Promise\PromiseInterface;
use function React\Promise\all;
class GetFullUsersInfoFromChannel extends Controller
{
/**
* @param Request $request
* @return \React\Promise\Promise|PromiseInterface|void
*/
public function __invoke(Request $request)
{
return all($this->channelManager
->getChannelMembers($request->appId, $request->channelName)
->then(function ($members) use ($request) {
return all([
'users' => all(collect($members)->map(function ($user) use ($request) {
$this->channelManager->getMemberSockets($user->user_id, $request->appId, $request->channelName)
->then(function ($sockets) use ($user) {
$user->sockets = $sockets; //resolved
return $user;
});
return $user; //resolved
})->values()->toArray())]);
}));
}
} and
can't be done in the |
hi, was this rejected or closed by mistake? |
2.x got closed |
As far as I checked out, there is a counter-pattern on your implementation, since you're trying to resolve the sync module in async by resolving all the promises. |
i first tried to make the FullfilledPromise JsonSeralizable but the reactphp guys in my PR lead me to this solution which works fine here. What would you suggest? |
this would basically re-enable the lost functionality of #150 |
Please see #735, it was a bit off to find out that custom routes accidentally got registered at runtime anytime the kernel ran, not just when the websocket command ran. |
@simonbuehler The only difference is that another function may be used to differ from the registration ones: WebSocketsRouter::addCustomRoute('GET', '/my-websocket', \App\MyCustomWebSocketHandler::class); |
hi,
this should fix #660