-
Notifications
You must be signed in to change notification settings - Fork 99
RFC: v5 Update #241
-
Hi
As noted in obs-websocket 4.9.1 release notes, the next release (5.0) will completely change the protocol. After question flew by in obs discord about 5.0.0 support @tt2468 reached out to @haganbmj if he's still working on this. As his interest in the project has dwindled they decided to transfer the repo to obs-websocket-community-projects and asked @Panger95, @t2t2 and @VodBox to maintain it.
Since the codebase has aged quite a bit and 5.0 will be a major release this gives a good chance to do a larger update.
I'll put the ideas as individual comments in this thread, but major changes will be converting the codebase to typescript, providing an esm/modern build and API changes (some as needed by 5.0 changes, others to clean up code). Please comment on anything you feel strongly about.
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
Replies: 6 comments 11 replies
-
Typescript conversion, packaging and bundling:
- Use microbundle for building/bundling
- Dependencies:
- events doesn't have the best typings, maybe eventemitter3 (seems to be smaller also)
- Check others for bundle size differences (sha.js vs crypto-js)
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Tested bundle sizes for sha256 calculations using vite to build following files: // src/crypto-js.ts import sha256 from 'crypto-js/sha256' import Base64 from 'crypto-js/enc-base64' const test1 = 'asd' const test2 = 'qwe' console.log('crypto', Base64.stringify(sha256(test1 + test2))) // src/sha-js.ts // @ts-expect-error Could not find a declaration file for module 'sha.js/sha256' (This is with @types/sha.js) import SHA256 from 'sha.js/sha256' const test1 = 'asd' const test2 = 'qwe' console.log('sha.js', new SHA256().update(test1).update(test2).digest('base64')) // src/sha-js-full.ts import SHA from 'sha.js' const test1 = 'asd' const test2 = 'qwe' console.log('sha.js', new SHA.sha256().update(test1).update(test2).digest('base64')) Also tried // vite.config.js import {resolve} from 'path' export default { build: { rollupOptions: { input: { 'crypto-js': resolve(__dirname, 'crypto-js.html'), 'sha-js': resolve(__dirname, 'sha-js.html'), 'sha-js-full': resolve(__dirname, 'sha-js-full.html') }, output: { manualChunks: false // By default compiles all modules in node_modules across different entrypoints to one vendor.js output file, which we don't want for this test } } } } Build: Since sha-js and sha-js-full both use
However when checking output there was one more issue: Turns out
So for best size and typescript support should switch from sha.js to crypto-js |
Beta Was this translation helpful? Give feedback.
All reactions
-
After posting this I did notice thread about webpack 5 warning about polyfilling crypto module: brix/crypto-js#354
This shouldn't really be an issue since it's fallback for getting secure number generator:
https://github.com/brix/crypto-js/blob/971c31f0c931f913d22a76ed488d9216ac04e306/src/core.js#L38
But since files aren't separated by browser and node usage it is throwing a waning. Option here is to either have instructions in readme about configuring webpack, or PRing a fix, but activity in that repo is also dead.
Looking through npm another option is jsSHA, stats:
// src/jssha.ts import jsSHA256 from 'jssha/dist/sha256' const test1 = 'asd' const test2 = 'qwe' const obj = new jsSHA256('SHA-256', 'TEXT') obj.update(test1) obj.update(test2) console.log('crypto', obj.getHash('B64'))
vite v2.3.7 building for production...
✓ 15 modules transformed.
dist/assets/favicon.17e50649.svg 1.49kb
dist/jssha.html 0.39kb
dist/crypto-js.html 0.39kb
dist/assets/crypto-js.377ea946.js 6.43kb / brotli: 2.41kb
dist/assets/jssha.6c2f99ad.js 9.54kb / brotli: 3.31kb
Beta Was this translation helpful? Give feedback.
All reactions
-
- events doesn't have the best typings, maybe eventemitter3 (seems to be smaller also)
I tried to use eventemitter3 but we cannot supply our own types there as that is required for the lib to function properlty
Beta Was this translation helpful? Give feedback.
All reactions
-
- events doesn't have the best typings, maybe eventemitter3 (seems to be smaller also)
I tried to use eventemitter3 but we cannot supply our own types there as that is required for the lib to function properlty
I think they fixed this, I may be wrong though
Beta Was this translation helpful? Give feedback.
All reactions
-
API Changes: Convert from async connect(args = {}) to async connect(address = 'ws://localhost:4444', password?: string, options?: IdentifyMessageFields)
More of a stylistic choice but would be small size decrease and closer to WebSocket params
Beta Was this translation helpful? Give feedback.
All reactions
-
Remove the hardcode of ws://localhost:4444 and instead change to an optional wss://localhost:4444 or ws://localhost:4444
Beta Was this translation helpful? Give feedback.
All reactions
-
there is a secure boolean atm that changes ws:// to wss://
Beta Was this translation helpful? Give feedback.
All reactions
-
I added the secure back then to support both secure and non-secure without breaking changes. But an address override is much better IMHO.
Beta Was this translation helpful? Give feedback.
All reactions
-
I think we have to keep the non-override as some systems like @t2t2 use the regular ws protocol I believe
Beta Was this translation helpful? Give feedback.
All reactions
-
The solution I came up with will prepend ws:// to the address if no prefix is present 5a91a3d
Beta Was this translation helpful? Give feedback.
All reactions
-
API Changes: Remove sendCallback
- Anyone who does want callbacks can use
.send(...).then((res) => { ... }, (error) => { ... })syntax. (whichsendCallbackdoes anyway) - Most modern codebases would async/await promises, making it bloat (and classes can't be treeshaken)
Beta Was this translation helpful? Give feedback.
All reactions
-
Removed in this commit 3c6f88d
Beta Was this translation helpful? Give feedback.
All reactions
-
Potentially make error logging more intuitive. Currently you will get a lot of uncaught exception errors because by default only errors on the initial socket connection will be caught. Any subsequent errors will need to currently be emitted to a catch all error handler and will be considered uncaught without this handler.
This may require a ton of additional code and I'm not sure how specific we should get with caught errors at the moment.
Beta Was this translation helpful? Give feedback.
All reactions
-
obs-websocket 5.0 protocol changes:
From attempting to implement a client, likely new API would be:
- internal?
message(messageType: string, data: Object)- to send base messages - public:
reidentify(data: Object)- to send Reidentify message - rename
sendtorequestorcall(last matches simpleobsws)?
And the rest from base eventemitter class for event related needs
Beta Was this translation helpful? Give feedback.
All reactions
-
Node.js: Follow Node.js LTS EOL schedule and up minimum version to v12
Beta Was this translation helpful? Give feedback.
All reactions
-
Completed in the typescript rewrite
Beta Was this translation helpful? Give feedback.