laravel / echo

Laravel Echo library for beautiful Pusher and Ably integration.
https://laravel.com/docs/broadcasting#client-side-installation
MIT License
1.18k stars 182 forks source link

feat: add echo options type to improve DX & use rollup to ship types #378

Closed dammy001 closed 7 months ago

dammy001 commented 1 year ago

… files

The bundle size has reduced also.

nandi95 commented 1 year ago

This is similar to https://github.com/laravel/echo/pull/356 😁

jessarcher commented 1 year ago

Thanks for the PR!

I'm all for adding the type definitions and eliminating as many any as possible!

There are quite a few additional opinionated changes in this PR, and some bundling changes that have me concerned, given how often stuff like this has broken peoples' apps.

For example, the IIFE changes have me worried that this PR reintroduces the issue solved in #344.

taylorotwell commented 1 year ago

Mark as ready for review when ready for me to review again please.

dammy001 commented 1 year ago

Thanks for the PR!

I'm all for adding the type definitions and eliminating as many any as possible!

There are quite a few additional opinionated changes in this PR, and some bundling changes that have me concerned, given how often stuff like this has broken peoples' apps.

For example, the IIFE changes have me worried that this PR reintroduces the issue solved in #344.

@jessarcher i get your point about the IIFE breaking change, I'll revert the declaration part.

dammy001 commented 1 year ago

@jessarcher please take a look at this again. potential IIFE issue has been reverted

jessarcher commented 1 year ago

@dammy001 There's still an awful lot going on in this PR unrelated to the PR title.

Is it possible to just add the options types without installing extra packages and reconfiguring the entire build process and module set-up?

driesvints commented 1 year ago

@dammy001 have you seen Jess' questions? Are you still working on this?

dammy001 commented 1 year ago

@driesvints I'll work on the changes today.

driesvints commented 7 months ago

@dammy001 feel free to resend this once you have more time