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

fix: Room not found #584

Merged
merged 11 commits into from
Jan 6, 2024
Merged

fix: Room not found #584

merged 11 commits into from
Jan 6, 2024

Conversation

MJinH
Copy link

@MJinH MJinH commented Jan 3, 2024

Fixes Issue

**My PR closes #577 **

πŸ‘¨β€πŸ’» Changes proposed(What did you do ?)

  1. commented out the lines in anonymous.tsx file. Had an issue where the socket would connect on the server side but immediately disconnect on the client side, which was causing the NEW_EVENT_CHAT_RESTORE event to not work properly.

  2. After debugging, noticed that the createChat function wasn't being called properly in the anonymous.tsx file. suspect this was because it was being called before the provider was initialized, so wrapped the components with ChatProvider in the _app.tsx file and confirmed that it was working correctly.

βœ”οΈ Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • This PR does not contain plagiarized content.
  • The title and description of the PR is clear and explains the approach.

Note to reviewers

πŸ“· Screenshots

Copy link

vercel bot commented Jan 3, 2024

Someone is attempting to deploy a commit to a Personal Account owned by @Dun-sin on Vercel.

@Dun-sin first needs to authorize it.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

PR Compliance Checks Passed!

@Dun-sin
Copy link
Owner

Dun-sin commented Jan 4, 2024

Will review this later in the day @MJinH in the mean time, please remove the package files, don't delete them but remove them from your PR. Thanks

Copy link
Owner

@Dun-sin Dun-sin left a comment

Choose a reason for hiding this comment

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

Leave the naming

Copy link
Owner

@Dun-sin Dun-sin left a comment

Choose a reason for hiding this comment

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

Please remove everything that doesn't have to do with the issue, like changing name or formatting?

Also why are you calling context twice as we've already called it in PageWrapper.tsx, so either you remove it from there and leave your changes or vice versa, but can't be both

@MJinH
Copy link
Author

MJinH commented Jan 4, 2024

Could you please check this? in the ChatContext.tsx file, when the name is "chatId" in the createChat, it seems to result in 'undefined' in the chatReducer as shown in the image. When using 'id', it looks like it's assigned correctly. Please let me know if i missed something. Also, yes you're right. I forgot to remove the context in the PageWrapper.tsx file.

Screenshot (211)

Screenshot (208)

@Dun-sin
Copy link
Owner

Dun-sin commented Jan 4, 2024

Could you please check this? in the ChatContext.tsx file, when the name is "chatId" in the createChat, it seems to result in 'undefined' in the chatReducer as shown in the image. When using 'id', it looks like it's assigned correctly. Please let me know if i missed something. Also, yes you're right. I forgot to remove the context in the PageWrapper.tsx file.

Screenshot (211)

Screenshot (208)

i guess the issue comes from here:
remove the ID and leave the chat Id, should fix it, can't believe i missed that
making it, also i would prefer the provider to be the in the pageWrapper:

chatId,

not thisπŸ‘‡

@MJinH
Copy link
Author

MJinH commented Jan 4, 2024

got it, and if the provider is in PageWrapper, the createChat in anonymous.tsx doesn't seem to be called properly. When debugging, it gets called as createChat: () => {} and exits immediately. can you please check this as well?

I'll fix those and recommit later. Thank you.

src/reducer/chatReducer.ts Outdated Show resolved Hide resolved
src/pages/api/server/index.ts Outdated Show resolved Hide resolved
src/pages/anonymous.tsx Outdated Show resolved Hide resolved
src/types/contextTypes.ts Outdated Show resolved Hide resolved
@Dun-sin
Copy link
Owner

Dun-sin commented Jan 4, 2024

Also @MJinH what exactly do you mean in Anonmyous.tsx?

@MJinH
Copy link
Author

MJinH commented Jan 4, 2024

What I mean is, when ChatProvider is in PageWrapper, createChat is exeucted like this
room-1.webm

and this is when the provider is in _app.tsx
room-2

@Dun-sin
Copy link
Owner

Dun-sin commented Jan 5, 2024

What I mean is, when ChatProvider is in PageWrapper, createChat is exeucted like this
room-1.webm

and this is when the provider is in _app.tsx
room-2

I see your point, yours is a better way thenπŸ’ͺ🏽 but the other things still count

Copy link

vercel bot commented Jan 5, 2024

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
whisper βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback Jan 6, 2024 7:45am

src/reducer/chatReducer.ts Outdated Show resolved Hide resolved
Copy link
Owner

@Dun-sin Dun-sin left a comment

Choose a reason for hiding this comment

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

You did great, thanks for contributing, I hope you will stick around and continue to contribute to this project.

Consider giving this project a star and joining the community discord server if you haven't for more resources and opportunities to connect with others. πŸ‘‰πŸ½hereπŸ‘ˆπŸ½

@Dun-sin Dun-sin merged commit 55b1794 into Dun-sin:main Jan 6, 2024
4 checks passed
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.

[BUG] Room not found
3 participants