saltyrtc / saltyrtc-client-js

SaltyRTC JavaScript implementation.
MIT License
29 stars 6 forks source link

Doesn't seem to work on Node.js and Deno #134

Closed rmst closed 2 years ago

rmst commented 2 years ago

I tried adapting the web example from https://github.com/saltyrtc/saltyrtc-demo for Node.js but it failed (expectedly). Is there a way to make Saltyrtc work in Node with https://github.com/node-webrtc/node-webrtc (and/or the Deno equivalent)?

dbrgn commented 2 years ago

Can you qualify "it failed"? 🙂 Were there any exceptions?

rmst commented 2 years ago

Yeah, I took https://github.com/saltyrtc/saltyrtc-demo/tree/master/web (which ran fine in the browser) and I modified the script.js so it can run via node script.js (removed dom references and added the dependencies in from the index.html as requires). The error is

ReferenceError: window is not defined
    at Object.1../adapter_factory.js (/Users/simon/dev/saltyrtc-demo/web/node_modules/webrtc-adapter/out/adapter_no_edge.js:15:62)

..from which I concluded is that the some of dependencies (in this case webrtc-adapter) aren't compatible with node.

For reference, this is what I used as my imports in script.js

require("tweetnacl/nacl-fast.min.js")
require("webrtc-adapter/out/adapter_no_edge.js")
require("msgpack-lite/dist/msgpack.min.js")
require("@saltyrtc/chunked-dc/dist/chunked-dc.es5.js")
require("@saltyrtc/client/dist/saltyrtc-client.es5.js")
require("@saltyrtc/task-webrtc/dist/saltyrtc-task-webrtc.es5.js")
dbrgn commented 2 years ago

The exception comes from adapter.js. Can you try to remove that import? It's only used as a compatibility shim for older browsers, things might work withou tit.

rmst commented 2 years ago

Without it I get

/Users/simon/dev/cerex-experiments/saltyrtc-demo/web/node_modules/@saltyrtc/client/dist/saltyrtc-client.es5.js:3289
}({},nacl,msgpack));
     ^

ReferenceError: nacl is not defined
    at Object.<anonymous> (/Users/simon/dev/cerex-experiments/saltyrtc-demo/web/node_modules/@saltyrtc/client/dist/saltyrtc-client.es5.js:3289:6)
dbrgn commented 2 years ago

What happens if you load the ES2015 variant (saltyrtc-client.es2015.js) instead of the ES5 variant? That file uses imports, so maybe this will fix the dependencies.

rmst commented 2 years ago
/Users/simon/dev/cerex-experiments/saltyrtc-demo/web/node_modules/@saltyrtc/client/dist/saltyrtc-client.es2015.js:31
import { box, randomBytes, secretbox } from 'tweetnacl';
^^^^^^

SyntaxError: Cannot use import statement outside a module

:P

rmst commented 2 years ago

Thanks for helping out, btw :) I'd make more of an effort to investigate this but I'm not super familiar with the intricacies of the node / js module system so I'm not sure what's going wrong here

dbrgn commented 2 years ago

Well, yes, if you use ES2015 modules you need a system that supports this out of the box, or a module bundler.

Unfortunately the JS ecosystem is one huge mess. It does get better, but really really slowly. I think I'd need to know what kind of project you have / how it's structured in order to help, otherwise it'll be hard. This library is currently optimized for the browser, and not for NodeJS, we've never tested it on Node.

rmst commented 2 years ago

There is no project structure beyond what I've said above – simply the web directory from the saltyrtc-demo. I tried changing script.js to script.mjs and various other things but everything just resulted in other import errors within the saltyrtc dependency. What's interesting to me is that you seem to expect this to work. Don't you rely on the browsers Webrtc implementation which by default isn't present in Node?

dbrgn commented 2 years ago

What's interesting to me is that you seem to expect this to work. Don't you rely on the browsers Webrtc implementation which by default isn't present in Node?

I don't know how https://github.com/node-webrtc/node-webrtc works, and assumed that - since you were trying this - it might contain a compatibility layer for the browser-based WebRTC API 🙂 In any case, the base saltyrtc-client-js library would probably work since it only requires msgpack-lite and tweetnacl. But if you bring WebRTC into play, it'll probably be a bit more complex.

I think for now I'll have to close this issue with "NodeJS is not currently supported". The library would require some restructuring for that to work.

lgrahl commented 2 years ago

With a build script for Node and a WebSocket compat shim, it should work. But we aren't going to make that effort. PRs are welcome though. :)