Replace uuid package with built-in crypto.randomUUID()
#1140
Conversation
|
This is not mergable, |
According to https://nodejs.org/api/crypto.html#cryptorandomuuidoptions, it's available since v14.17.0... :-/ |
|
Please use reviews for comments both ways 🙏 I have 7 notifications already within minutes here. |
|
CI fails because |
piranna
force-pushed
the
uuidv4
branch
2 times, most recently
from
690cf82 to
fd8968f
Compare
|
I've fixed all the concerns.
|
- Add `node:` prefix to all built-in Node imports (replaces PR #1142). - Replace `uuid` package with built-in `crypto.randomUUID()`(replaces PR #1140). - Cosmetic changes in TS files: - Avoid unnecessary `const { createWorker } = mediasoup` in almost all tests. - Move all FBS related imports to the bottom. - Some formating fixes that bypassed our ESLint rules.
|
Replacing this PR with #1193 which targets |
- Add `node:` prefix to all built-in Node imports (replaces PR #1142). - Replace `uuid` package with built-in `crypto.randomUUID()`(replaces PR #1140). - Cosmetic changes in TS files: - Avoid unnecessary `const { createWorker } = mediasoup` in almost all tests. - Move all FBS related imports to the bottom. - Some formating fixes that bypassed our ESLint rules.
This PR replace
uuidpackage with Node.js built-incrypto.randomUUID()function. Also, it fixes a couple of typos where the random UUID would be generated if an empty string would be provided asid, now only it's being generated ifundefinedis provided (note, we should validate the providedidis an UUID instead of a random string, though).OTOH, I've found that the
idfield ofDataProducerOptionsis not documented. Not fully sure if it makes sense to provide it all, since bandwidth and CPU are low, or it's just there only to mimicProducerAPI...