-
Notifications
You must be signed in to change notification settings - Fork 50
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 payload serialization #115
Comments
I just tested using Laravel's SerializableClosure, but I don't think it's gonna work, unless we change the API. For this package to work, the API would have to be something like this: $todo->broadcastAppend()
->view(fn () => view('todos._todo', ['todo' => $todo]))
->later(); Notice the $data = [
'todo' => $todo,
];
serialize(new SerializableClosure(fn () => view('todos._todo', $data))); We could probably handle the default behavior inside the package itself (serializing the model correctly. Also, using the package we probably wouldn't be able to dispatch a queued job in a REPL context like |
@tonysm The call to the |
Not sure I get your suggestion. 🤔 The issue is not about the broadcast methods themselves. The problem is only when we pass custom views with data since we need to serialize that to render on the queued job. If we're only broadcasting the model itself, then this is not an issue, since it serializes just fine. |
I think Laravel's new Defer helpers would work here instead of pushing rendering to a queued job... maybe the entire "later" publishing could use defer instead of queued jobs. |
When we send a broadcasting job to be processed in background, because the job payload can be anything (whatever you want to pass to the view), we don't really use Laravel's
ModelIdentifier
class (here).The current implementation works, but the jobs are unnecessarily big, since the object is serialized using PHP's normal serialization. This also has the side-effect of the model having all the attributes it had during from the moment the model was serialized. For instance, if we wanted to return the HTML slightly modified as an HTTP response Turbo Stream for the user that created the resource, we'd use the
$model->wasRecentlyCreated
, but if we wanted the broadcasted Turbo Stream to not be modified, we would have to override the data passed to the view, because since the model was serialized, it will also have the$model->wasRecentlyCreated
set totrue
when the broadcast job is processed.I think this could be solved by walking through the data array and finding out the eloquent models and either converting them to either use
ModelIdentifier
or maybe usingglobalid-laravel
(here).The text was updated successfully, but these errors were encountered: