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

Use STOMP on top of websockets to receive events in real time #8

Merged
merged 11 commits into from
Apr 5, 2022

Conversation

IncPlusPlus
Copy link
Owner

@IncPlusPlus IncPlusPlus commented Apr 5, 2022

In trying to figure out what was the best way to use react-stomp in the frontend such that we could have a straightforward way to subscribe to all the topics relevant to each of the towers the user is a member of. What I settled on was sneakily using the <SockJsClient> component in each element of the TowerList component. I extracted what was a simple <li> entry into its own component called TowerListElement. This still returns a simple <li> as part of a React fragment (<> </>) that also contains <SockJsClient> (which doesn't render any visible content so it doesn't mess anything up).

Just like #7, this includes a lot of changes but doesn't fundamentally change anything about the structure of the app. The only really big change is TowerListElement. Like IncPlusPlus/beacon-backend#34, I suggest you look at the changes individually per commit instead of all at once since this is kind of big.

Also, feel free to view the deployment created for this PR. Just know that it won't work properly unless EITHER

  1. You set the cisBasePath local storage value to https://beacon-cis-pr-34.herokuapp.com/ OR
  2. beacon-backend#34 is merged

Something you'll notice in the console from now on is the following warning. This is a known issue with react-stomp. See lahsivjar/react-stomp#173.
image

If you're curious about the messages being passed around and how to see them, open up the Network tab of your browser's dev tools and refresh the page. Look for the one with the name "websocket" and click on it. If you don't see the websocket request, you may have opened the dev tools after the connection was already established, in which case you can just reload the page and the connection will be re-established before your very eyes.

image

Next, click the Messages tab.

image

You can now click on any message to see its content.

Once again, please ask questions about literally anything.

This makes its purpose a lot more clear.
No more waiting 10 seconds before anything shows up
Instead of calling towers.get(towerId) a bunch of time, only bother calling it once and hanging onto that instance of ObservableTower.
We never have a list of messages to provide to ObservableChannel at construction time, nor do we ever have a list of channels to pass to an ObservableTower at construction time. As such, they shouldn't exist as construction parameters.
Apparently, the place where you call makeAutoObservable in the constructor is really important. I was calling it at the beginning but since the fields were technically uninitialized at that point, the values were never being observed properly and re-renders weren't happening even when the state changed (such as when a message was edited).
Channels will now display in order of their order property. The ordering will be from the lowest number to highest, from top to bottom such that channels at the top have the lowest order number and those at the bottom have the highest.
@IncPlusPlus IncPlusPlus self-assigned this Apr 5, 2022
@IncPlusPlus IncPlusPlus temporarily deployed to beacon-frontend-pr-8 April 5, 2022 21:12 Inactive
Copy link
Collaborator

@Sciman101 Sciman101 left a comment

Choose a reason for hiding this comment

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

Looks good

@IncPlusPlus IncPlusPlus merged commit 836d409 into main Apr 5, 2022
@IncPlusPlus IncPlusPlus deleted the experiment/websockets branch April 5, 2022 21:58
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.

2 participants