Skip to content

Conversation

@vakiliner
Copy link
Contributor

@vakiliner vakiliner commented Oct 16, 2025

Please describe the changes this PR makes and why it should be merged:

Closes #11180

Status and versioning classification:

  • 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

@vercel
Copy link

vercel bot commented Oct 16, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
discord-js Ignored Ignored Preview Oct 30, 2025 3:16pm
discord-js-guide Ignored Ignored Preview Oct 30, 2025 3:16pm

@vakiliner vakiliner changed the title fix: ClientReady is called without user and application fix(Client): Move client initialization code Oct 16, 2025
@vakiliner vakiliner marked this pull request as ready for review October 16, 2025 15:22
@vakiliner vakiliner requested a review from a team as a code owner October 16, 2025 15:22
@Qjuh
Copy link
Member

Qjuh commented Oct 19, 2025

I'd say the better fix would be to move all the READY event handling into READY.js, not the other way around. Or at least the checkReady() call

@vakiliner
Copy link
Contributor Author

I'd say the better fix would be to move all the READY event handling into READY.js, not the other way around. Or at least the checkReady() call

I just noticed that the Ready event is async
None of the packet handlers are async, so calling this function won't be awaitable
While this can easily be fixed by adding "await" before "PacketHandlers[packet.t](this, packet, shardId);", but is it necessary?

@vakiliner
Copy link
Contributor Author

I'd say the better fix would be to move all the READY event handling into READY.js, not the other way around. Or at least the checkReady() call

I believe this was not added to READY.js on purpose because the handler is async

@Qjuh
Copy link
Member

Qjuh commented Oct 19, 2025

I'd say the better fix would be to move all the READY event handling into READY.js, not the other way around. Or at least the checkReady() call

I believe this was not added to READY.js on purpose because the handler is async

Since I was the one who wrote that I can assure you that's not the reason. In v14 the checkReady check is in READY.js; this was simply an oversight when adapting to the direct use of /ws and should be reverted to check in READY.js

adding await would be a good idea however, and then the checkReady check for READY event could be added to handlePacket instead. Similar to how it checks for GUILD_CREATE and _DELETE.

@vakiliner
Copy link
Contributor Author

In v14 the checkReady check is in READY.js

Then checkReady() was not async

@vakiliner vakiliner requested a review from Qjuh October 21, 2025 17:27
Copy link
Member

@didinele didinele left a comment

Choose a reason for hiding this comment

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

before merging, we should give this a little test.

@Jiralite would you mind? otherwise, I can get to it tonight
discord.js@15.0.0-move-client-init.1761650119-a4c0a246f

@didinele
Copy link
Member

was a little later than "tonight", but I got to it. we should be good :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

ClientReady is called without user and application

4 participants