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

feature: public raw events #3159

Merged
merged 7 commits into from
Apr 29, 2019
Merged

Conversation

bdistin
Copy link
Contributor

@bdistin bdistin commented Mar 20, 2019

Please describe the changes this PR makes and why it should be merged:
This gives a public alternative to the private raw event. (The raw event isn't removed, for the purposes of library testing.) This gives devs the ability to work with d.js in a cacheless way without relying on the private raw event. Say you don't want to cache partial reactions simply to have a starboard type feature that isn't limited to your message cache, this would give you that public option to do so.

Like raw, these event's aren't disabled with ClientOptions#disabledEvents (which isn't very aptly named, since it only disables the internal d.js handlers). So you could disable all d.js handlers (and by proxy all d.js caching) and use these events to cache to Redis (microservices like scenario), or any other number of completely custom use cases.

Usage is very simple: Client#ws.on(RawEventType, (RawEventData, shardID) =>{});

This was originally suggested by @appellation as an alternative to raw/partials quite a while back. I liked the idea back then, however, the idea didn't get much traction as most people thought partials would offer what people were using the raw event for. It seems though, this idea still holds a niche that partials didn't or can't offer.

If this pr gets support, the docs/typings could be greatly improved with more detail of the api data structures. Or simply we could link to discordapp docs for each event type. I only did the basics so that this pr could be made with little wasted effort if the idea is not liked. It appears documenting specifics is something that is specifically unwanted.

Status

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

Semantic versioning classification:

  • This PR changes the library's interface (methods or parameters added)
    • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
  • This PR only includes non-code changes, like changes to documentation, README, etc.

while retaining raw for use in debugging privately
@@ -1277,6 +1277,76 @@ declare module 'discord.js' {
public status: Status;

public broadcast(packet: object): void;

public on(event: 'READY', listener: (data: any, shardID: number) => void): this;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can greatly simplify those overloads via the WSEventType type:

public on(event: WSEventType, listener: (data: any, shardID: number) => void): this;

public once(event: WSEventType, listener: (data: any, shardID: number) => void): this;

@SpaceEEC SpaceEEC dismissed their stale review March 20, 2019 19:26

Requested changes were applied.

@@ -1305,6 +1305,10 @@ declare module 'discord.js' {
public status: Status;
public readonly ping: number;

public broadcast(packet: object): void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed, as it's already mentioned in the private functions part of the class

@amishshah amishshah merged commit 9b0f4b2 into discordjs:master Apr 29, 2019
@bdistin bdistin deleted the PublicRawEvents branch April 29, 2019 18:16
samsamson33 pushed a commit to samsamson33/discord.js that referenced this pull request Feb 27, 2020
* add a public alternative to the private raw event

while retaining raw for use in debugging privately

* only emit dispatch packets

* requested changes

TIL, that's neat

* fix padding

* requested changes

* Update WebSocketManager.js
samsamson33 added a commit to samsamson33/discord.js that referenced this pull request Feb 27, 2020
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.

5 participants