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

[flatbuffers] Improve Node imports #1193

Merged
merged 1 commit into from
Oct 25, 2023
Merged

Conversation

ibc
Copy link
Member

@ibc ibc commented Oct 25, 2023

- 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.
export function generateUUIDv4(): string
{
return randomUUID();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is that? Why not using directly the node:crypto export?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make it clear that it's a UUID version 4 and to be consistent with other utils such as generateRandomNumber() which also uses crypto API inside. It's not bad to have wrappers that allow us to change the internal implementation at any time if needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not my taste of cake, but ok 👍🏻

@ibc ibc merged commit 53dbfce into flatbuffers Oct 25, 2023
@ibc ibc deleted the flatbuffers-improve-node-imports branch October 25, 2023 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants