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

feat: support uWebSocket.js #22

Closed
wants to merge 13 commits into from
Closed

feat: support uWebSocket.js #22

wants to merge 13 commits into from

Conversation

rtritto
Copy link
Contributor

@rtritto rtritto commented Oct 21, 2024

Changes/TODO

Fix #20

Note: Help and reviews are needed

@rtritto rtritto changed the title feat: init uWebSocket.js support feat: support uWebSocket.js Oct 21, 2024
@rtritto
Copy link
Contributor Author

rtritto commented Oct 21, 2024

FYI @nitedani

@rtritto
Copy link
Contributor Author

rtritto commented Oct 22, 2024

https://github.com/vikejs/vike-node/actions/runs/11464915212/job/31902213911?pr=22

Error: 42.542][/test/vike-node/.prod-uws.test.ts][npm run prod][stderr] ✘ [ERROR] No loader is configured for ".node" files: ../../node_modules/.pnpm/uWebSockets.js@https+++codeload.github.com+uNetworking+uWebSockets.js+tar.gz+442087c0a01bf146_ef5o3ypzxljf3ryz2pfwb3ukxu/node_modules/uWebSockets.js/uws_linux_arm_108.node

What should we use to load native Node (.node) modules?

Some alternatives:

There is a vike plugin to do that?

FYI @brillout @nitedani

@rtritto
Copy link
Contributor Author

rtritto commented Oct 23, 2024

https://github.com/vikejs/vike-node/actions/runs/11464915212/job/31902213911?pr=22

Error: 42.542][/test/vike-node/.prod-uws.test.ts][npm run prod][stderr] ✘ [ERROR] No loader is configured for ".node" files: ../../node_modules/.pnpm/uWebSockets.js@https+++codeload.github.com+uNetworking+uWebSockets.js+tar.gz+442087c0a01bf146_ef5o3ypzxljf3ryz2pfwb3ukxu/node_modules/uWebSockets.js/uws_linux_arm_108.node

What should we use to load native Node (.node) modules?

Some alternatives:

There is a vike plugin to do that?

FYI @brillout @nitedani

I fixed using uws-pack that uses dynamic import to import the current Node.js native module.

Now /test/vike-node/.dev-uws.test.ts got timeout: https://github.com/vikejs/vike-node/actions/runs/11485389164/job/31965334594?pr=22

@brillout
Copy link
Member

brillout commented Nov 1, 2024

I'll have a look at this PR after I'm done with my current backlog. ETA soon.

@rtritto
Copy link
Contributor Author

rtritto commented Nov 25, 2024

Note: this PR can be used (generalized common factor) for all HTTP server frameworks that didn't use Node.js objects (Request, IncomingMessage, ServerResponse)

@brillout
Copy link
Member

@rtritto Thank you for your PR and the ping. It would indeed be great to support uWebSockets. In #16 we're starting using universal-middleware for vike-node. In other words: some(/most?) of the code in this PR would need to be moved to universal-middleware.

Note

In case you're curious: the vision is to have powerful Vike extensions that can fully integrate with each other automatically (with eject if the user wishes more control).

@magne4000 WDYT? On one hand it would quite nice to support uWebSockets and on the other hand if it slows us down too much then maybe it isn't worth it.

@magne4000
Copy link
Member

uWebSockets would indeed need to be integrated into universal-middleware instead of vike-node. @rtritto perhaps that's something you'd be willing to try?

@rtritto
Copy link
Contributor Author

rtritto commented Nov 26, 2024

@magne4000 I don't have (yet) the knowledge to create a new adapter for universal-middleware; as my initial analysis, @universal-middleware/core needs to be adapted (tell me if I'm wrong).

Maybe @hattip/adapter-uwebsockets can help.

Anyway you can start a PR and then ping me or ask for reviews or help.

@brillout
Copy link
Member

In the meantime, let's close this PR in favor of a potentially upcoming universal-middleware PR. Thanks again @rtritto for the groundwork 💯

@brillout brillout closed this Nov 26, 2024
@magne4000
Copy link
Member

@magne4000 I don't have (yet) the knowledge to create a new adapter for universal-middleware; as my initial analysis, @universal-middleware/core needs to be adapted (tell me if I'm wrong).

Probably not. Creating a new @universal-middleware/uwebsockets should be enough.

I'll let you know when I'll start working on it, but it's not on my priority list right now.

@rtritto rtritto deleted the uws branch November 27, 2024 12:37
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.

[Feature] Support to uWebSockets.js
3 participants