Fix types for the echo library

[dennisprudlo]

I am using echo in a TypeScript project and I have to mark a lot of function calls with // @ts-ignore since the library uses types that don't always refer to the actual type being used.

The whisper function for example spits out a type error, since the private()-function has the abstract Channel as a return type. Channel however doesn't have a whisper function.

Echo.private(channelName).whisper(...);

I have made some changes so the Echo class is more generic based on what is passed as a broadcaster in the options. I don't know if this is the best or the most elegant way to specify the types but with these changes the functions actually return the types they are supposed to:

const reverb = new Echo({ broadcaster: 'reverb' });

reverb.private(...) // PusherPrivateChannel
reverb.join(...) // PusherPresenceChannel

const socketio = new Echo({ broadcaster: 'socket.io' });

socketio.private(...) // SocketIoPrivateChannel
socketio.join(...) // SocketIoPresenceChannel
socketio.join(...).whisper(...) // <- TypeScript knows of this function and doesn't complain about an undefined function anymore.

I split the PR in separate commits to better understand the changes.

1. Some function of the connectors already had the correct return type, others didn't. The socket.io connector had the right types, the pusher connector didn't and the null connector only had the wrong type for the presence channel. With "wrong type" I mean a "not specifc enough" type. The return type was always the abstract type shared by all connectors.
2. This adds the generic nature for the functions.
3. Throws an error when attempting to use the encryptedPrivate() function with the socket.io broadcaster. This would throw an Error in any way, the addition is only to satisfy TypeScript.
4. Adds a fix for using a function as a broadcaster. The connector needs to be any since leaveAllChannels() expects a channels property on the connector that the base Connector doesn't have.

There are some other thing like when calling the following:

const reverb = new Echo({ broadcaster: 'reverb' });

reverb
    .private(...) // PusherPrivateChannel
    .listen() // PusherChannel

the listen() function returns a PusherChannel again while it should be a PusherPrivateChannel. But I wanted to see if there is actually any interest in these changes before tackling this one.

[dennisprudlo]

Will have a look at the npm install error once feedback arrives on the basic idea. The StyleCI issues show some expected changes that doesn't match with what the echo.ts looked like before, so I am not sure about them.

[taylorotwell]

Hey Dennis! Would need to lean on your expertise here. I am not a TS expert by any means. Feel free to push through the errors and proceed. Would be happy to get this merged. Please mark as ready for review when the changes have been made.

[dennisprudlo]

Ok, so regarding the StyleCI issues, still not sure. It recommends changing the coding style for areas that already were styled like this, but I am happy to update this as well to satisfy StyleCI.

A few things here: Due to the nature of the code a few more things needed to be generic and since TypeScript is TypeScript, once you started with generics it wants you to go all the way.

Most of the changes are only types, these should not have an effect on any logic of Echo. However a few other changes were necessary:

1. Exception
As I said I had to throw an exception explicitly to satisy typescript in the encryptedPrivate() function in echo.ts since socket.io doesn't support that. JavaScript should have exploded here any way since the called function in there doesn't even exist in the SocketIoConnector. It's hard to create a test for it though since I need to check if the correct error is being thrown, however all drivers throw other errors before that so jest tells me that the expected error message is different.

2. Public Channels
Up until now the PusherChannel, SocketIoChannel and NullChannel classes were used for public channels since they include all the logic. The private channel classes only add some logic to it. Now these channels need to be generic (for example it can be a PusherChannel<PusherPresenceChannel>) therefore the broadcasters now got a new empty PusherPublicChannel class which is used for the types. It doesn't add any logic but simply extends PusherChannel<PusherPublicChannel>. Nonetheless it is an additional class layer which will also be existing in the "bundled JavaScript world".

3. NullEncryptedPrivateChannel
Additionally a NullEncryptedPrivateChannel class has been added (same implications as in 2.) to streamline the types. The null broadcaster was able to call the encryptedPrivate() function but got a NullPrivateChannel out of it while pusher had a designated PusherEncryptedPrivateChannel. This is not necessary per se but the type is now more explicit. Also, when some additional encryption functionality will be necessary the logic can be added to this class instead of the NullPrivateChannel (which is used for non-encrypted private channels as well).

4. as unknown as T
Since the code by itself technically allows to build non-sensical constructs, the methods in the base channels cannot simply return this. In this case we have to force TypeScript in a quite dirty way to the correct type with return this as unknown as T. If there is anyone who knows how to clean that up – I'm all ears.

5. Tests
One of the tests checks if passing an invalid broadcaster throws the expected exception. But typescript now forces us to use a valid broadcaster. The new Echo({ broadcaster: 'foo' }) is invalid code in any typescript project. To still check if the exception is thrown I had to ignore the typescript error and the following eslint error.

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.

---

@taylorotwell I am not a TypeScript Typesystem expert as well but from my tests the functions now return the types correctly and carry them on through different classes.

Would love to hear some feedback from some more experienced type enthusiasts if anyone has the time for it.

[wking-io]

Going to start with this one and add more as I continue to review.

The use of generics for channels is not necessary thankfully. By using this as the return type we can get a really nice fluent API with good types an no overhead. It automatically types it as the most derived type automatically.

Example to apply to all channel code:

abstract class Base {
  abstract someMethod(): this;

  anotherMethod(): this {
    // Base logic
    return this;
  }
}

class FirstLevel extends Base {
  someMethod(): this {
    // Logic specific to FirstLevel
    return this;
  }

  firstLevelMethod(): this {
    // Logic specific to FirstLevel
    return this;
  }
}

class SecondLevel extends FirstLevel {
  someMethod(): this {
    return this;
  }

  firstLevelMethod(): this {
    return this;
  }

  secondLevelMethod(): this {
    // Logic specific to SecondLevel
    return this;
  }
}

[wking-io]

Will double check this tomorrow, but that should help clean a lot of this up to be more clean and avoid generics as much as we can.

[wking-io]

You can make these a little more helpful.

abstract class Connector<
  TPublic extends Channel,
  TPrivate extends Channel,
  TPresence extends Channel & PresenceChannel
>

This gives us compile time errors if we try to call connector with classes that aren't extensions of a channel.

Also ensures that the Presence generic implements the interface.

[wking-io]

An interface can also use the this return so that we can avoid generics.

[taylorotwell]

Drafting while this is actively worked on. Please mark as ready for review when you want me to take another look.

[dennisprudlo]

@wking-io Hey, many thanks! I didn't think of simply returning the current instance as a type, this definitely cleans up a lot of that generic mess. Most methods now return this so it's clear that the instance itself is being returned and not only something of the same type. I would love for you to have another look.

For the connector I used TPresence extends PresenceChannel for the presence channel generic, since PresenceChannel already extends Channel.

We also could specify that a function broadcaster must return Connector<Channel, Channel, PresenceChannel> if we wanted to. I am just not sure if it is necessary, since function broadcasters aren't documented anyway.

[wking-io]

Happy to be of assistance! Everything else is looking great 🫡

[dennisprudlo]

Ok, most of the generics could be removed, as well as other intermediate classes that were necessary for the "generic solution". Much cleaner now.

[jshah4517]

6. Function Broadcaster
   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.

This looks to have led to an (unintended?) breaking change.

Our old code for the broadcaster was just Connector (i.e. the connector class) and now it is (options) => new Connector(options).

Update: it was fixed in #404

[jordanhavard]

For anyone running into ts generic decleration issues, I found adjusting the below worked well for me in my types/index.d.ts file

import Echo from "laravel-echo";

declare global {
  interface Window {
-    Echo: Echo;
+    Echo: InstanceType<typeof Echo>;
  }
}