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

Missing type definitions for PresenceChannel #317

Closed
JonathanGuo opened this issue Jul 12, 2021 · 4 comments
Closed

Missing type definitions for PresenceChannel #317

JonathanGuo opened this issue Jul 12, 2021 · 4 comments

Comments

@JonathanGuo
Copy link
Contributor

JonathanGuo commented Jul 12, 2021

  • Echo Version: 1.11.0
  • Laravel Version: 8.49.2
  • PHP Version: 8.0.6
  • NPM Version: 6.14.8
  • Node Version: 14.15.1

Description:

First of all, thanks for the great work of the laravel-echo package!

I've noticed the type definition of PresenceChannel is incorrect. (it might be out of date).

According to the interface PresenceChannel it has only 3 functions:

/**
 * This interface represents a presence channel.
 */
export interface PresenceChannel {
    /**
     * Register a callback to be called anytime the member list changes.
     */
    here(callback: Function): PresenceChannel;

    /**
     * Listen for someone joining the channel.
     */
    joining(callback: Function): PresenceChannel;

    /**
     * Listen for someone leaving the channel.
     */
    leaving(callback: Function): PresenceChannel;
}

However, the PusherPresenceChannel extends PusherChannel, which has more functions. e.g. listen

Functionally it's working fine with compiled JavaScript package, but with the @typescript-eslint rule no-unsafe-call rule turned on, we will get the warning/error (depends on config) messages:

Unsafe call of an `any` typed value.eslint@typescript-eslint/no-unsafe-call

And the following error from ts:

Property 'listen' does not exist on type 'PresenceChannel'.ts(2339)

Unfortunately, we cannot even cast the type because the types/interfaces do not overlap:

(Echo.join(channelName) as Channel).listen() // Would throw the following error

// Conversion of type 'PresenceChannel' to type 'Channel' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
// Type 'PresenceChannel' is missing the following properties from type 'Channel': options, listen, listenForWhisper, notification, and 4 more.ts(2352)

🚑 Hotfix

To who has similar issues and seek a workaround, use double-cast can be a quick way:

(Echo.join(channelName) as unknown as Channel).listen() 

Steps To Reproduce:

  1. Turn on the rule @typescript-eslint/no-unsafe-call
  2. Chain the presence channel: Echo.join(channelName).listen(() => {});

I am more than happy to submit a PR to fix the interface definitions. Please let me know if the issue I raised is on the right track.

@driesvints
Copy link
Member

I don't really see an issue here, sorry. A presence doesn't needs all these methods. They're defined on the base Channel class.

@JonathanGuo
Copy link
Contributor Author

I don't really see an issue here, sorry. A presence doesn't needs all these methods. They're defined on the base Channel class.

Hi @driesvints . Surely the errors exist in TypeScript. The compiled JavaScript code has the methods, but the PresenceChannel Interface misses the definition of the methods.

The following examples are created in this repo:

First, it doesn't have a type hint when using TS:

image

It has a Property does not exist on Type error when you use listen
image

Secondly, it cannot be cast to Channel due to the interface doesn't extend anything, they don't overlap the type definitions:
image

This issue is basically caused by missing type definitions.

Please review.

@driesvints
Copy link
Member

It's best you attempt a PR if you think anything is broken here 👍

@JonathanGuo
Copy link
Contributor Author

It's best you attempt a PR if you think anything is broken here 👍

Will do

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

No branches or pull requests

2 participants