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

Document the Socket.io API and try to convert it to a HTTP request #3854

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

louislam
Copy link
Owner

@louislam louislam commented Oct 7, 2023

Resolves: #118
Resolves: #1210
Resolves: #1276
Resolves: #2358 (might be debatable)
Resolves: #2553 (might be debatable)
Resolves: #3060
Resolves: #3668

#1834 is staled as I think that the workload of implementing and maintaining both rest api and socket.io api is too heavy for me.

New approach:

  • Document the Socket.io API, people can connect to it via other Socket.io clients.
  • Since Socket.io client is not available for all programming languages, make an endpoint /api to convert a HTTP request to socket.io request
  • Define all possible requests in a JSON file
    • /api can use it to verify the request
    • Generate human readable docs from this json file

Of course, the API is not as beautiful as a REST API, but for me, it is much easier for me to maintain. It should work for most "crud" actions.

@louislam louislam added this to the 2.0.0 milestone Oct 7, 2023
@CommanderStorm

This comment was marked as outdated.

@chakflying
Copy link
Collaborator

If we do it this way, we would have to be clear on the performance expectations, I suspect having to open a new websocket connection inside the server on every api call would be quite a burden.

@louislam
Copy link
Owner Author

louislam commented Oct 8, 2023

If we do it this way, we would have to be clear on the performance expectations, I suspect having to open a new websocket connection inside the server on every api call would be quite a burden.

I am reading socket.io's test, it seems that it is possible to create a socket object without an actual real websocket connection.
https://github.com/socketio/socket.io/blob/main/test/socket.ts

@louislam
Copy link
Owner Author

louislam commented Oct 9, 2023

If we do it this way, we would have to be clear on the performance expectations, I suspect having to open a new websocket connection inside the server on every api call would be quite a burden.

I am reading socket.io's test, it seems that it is possible to create a socket object without an actual real websocket connection. https://github.com/socketio/socket.io/blob/main/test/socket.ts

Ah... It isn't what I thought, createClient is not a builtin function and it is actually a websocket connection

https://github.com/socketio/socket.io/blob/b4dc83eb9b85e26f09f25e1b5320fe3f49b521d3/test/support/util.ts#L33C1-L41

@louislam louislam modified the milestones: 2.0.0, 2.1.0 Oct 14, 2023
@louislam louislam mentioned this pull request Oct 14, 2023
@louislam louislam mentioned this pull request Dec 3, 2023
9 tasks
@CommanderStorm CommanderStorm added the area:api concearning the api or automation label Dec 5, 2023
@CommanderStorm CommanderStorm added the needs:work this PR needs work label May 19, 2024
@github-actions github-actions bot added needs:resolve-merge-conflict A merge-conflict needs to be addressed before reviewing makes sense again and removed needs:resolve-merge-conflict A merge-conflict needs to be addressed before reviewing makes sense again labels May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api concearning the api or automation needs:resolve-merge-conflict A merge-conflict needs to be addressed before reviewing makes sense again needs:work this PR needs work
Projects
None yet
3 participants