Add support for node 20 and drop support for node < 18 #2827
Conversation
xuesichao
force-pushed
the
node20
branch
2 times, most recently
from
5c10d02
to
629b521
Compare
src/utils/Utils.ts
Outdated
| * Returns a Crypto object, either from the window or from the node:crypto package. | ||
| * @returns Get the crypto object from the global scope. | ||
| */ | ||
| export function getCrypto(): Crypto { |
There was a problem hiding this comment.
When running unit tests, it's node env not browser env, we need this to be able to pass the unit test.
Check https://nodejs.org/api/crypto.html#determining-if-crypto-support-is-unavailable
There was a problem hiding this comment.
We do not support Node enviroment though. In our unit test can we catch the error and just return math.random instead?
There was a problem hiding this comment.
We did mock crypto.getRandomValues() here:
But the problem I'm having now is, when running unit test using node 20, it throws this error:
TypeError: Cannot set property crypto of #<Object> which has only a getter
at new DOMMockBuilder (test/dommock/DOMMockBuilder.ts:1525:21)
at Context.<anonymous> (test/messagingsession/MessagingSessionConfiguration.test.ts:16:22)
at processImmediate (node:internal/timers:478:21)
I'm trying to resolve this and then we should be good.
There was a problem hiding this comment.
Hmm so maybe we can just check on whether window.crypto exists, log a warning and fall back to Math.random instead? We do not support Node so kinda prefer just fallback to something else.
…his requires use of the new node:crypto package which was added
xuesichao
force-pushed
the
node20
branch
2 times, most recently
from
04d8a6c
to
8bb62cf
Compare
| "webpack": "^5.89.0", | ||
| "webpack-cli": "^4.8.0" |
There was a problem hiding this comment.
need to update the webpack version to avoid this issue:
webpack/webpack#14532
The webpack hashing algorithm uses node:crypto
Issue #2825:
Description of changes:
Add support for node 20 and drop support for node < 18
Testing:
Verified
npm installworks for demos and sdks. Tested meeting demo e2e.Node 18
Node 20
Can these tested using a demo application? Please provide reproducible step-by-step instructions.
N/A
Checklist:
Have you successfully run
npm run build:releaselocally?Yes, succeeded.
Do you add, modify, or delete public API definitions? If yes, has that been reviewed and approved?
No
Do you change the wire protocol, e.g. the request method? If yes, has that been reviewed and approved?
No
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.