Skip to content
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 custom broadcaster usages #404

Merged
merged 4 commits into from
Nov 19, 2024

Conversation

bytestream
Copy link
Contributor

Support for custom broadcasters was added in #247 but usage wasn't documented, and tests weren't added.

In 820ab6e the way custom broadcasters can be implemented changed, due to the removal of the new keyword. This was raised on the PR:

6. Function Broadcaster
This is a bigger question mark for me. The tests show that a simple function can be used as a broadcaster. The code however calls this function and attempts to construct whatever comes out of it. I am not sure where function broadcasters are used but I couldn't find any documentation on it. The problem here is that TypeScript complains and says "Since you are using new broadcaster(), whatever comes out of broadcaster must be a constructable". But the function that is passed as an example in the tests is not a constructable. So I can either satisfy the tests or the types.

In my understanding the function broadcaster enables users to write any kind of function that creates a custom connector and Echo will then use this connector. In this case I don't think the new is necessary here, so I removed it. At the same time the connector and channel types for the function broadcaster are now any. We would assume that the custom connector will have the same methods as other connectors but technically the user could return 'foobar'; in there and we will never know for sure.

I've fixed the type errors and reverted the removal of the new keyword, so that usages remain as before.

@taylorotwell taylorotwell merged commit d7f2021 into laravel:master Nov 19, 2024
4 checks passed
@bytestream bytestream deleted the fix-custom-broadcaster branch November 19, 2024 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants