← Back to context

Comment by toraway

20 hours ago

Shouldn't your test follow the pattern of how rng() is actually being used in the uuid.ts code internally?

Your test is more-or-less contrived to fail given the tradeoff to avoid repeated memory allocations but that doesn't say much about the actual usage in uuid generation since it's not exported for general purpose use.

Presumably they had some hot path somewhere where rng() is called in a loop and this optimization made sense with awareness that it could be misused as in your example breaking the contract ensuring randomness, which (hopefully) they're not actually doing anywhere.

Unless I'm missing something replacing the package over this with a less vetted implementation seems excessive and possibly even counterproductive.

I don't believe so. Sure it's not an issue after some checks, but it's very easy to shoot yourself in the foot like that. I get the micro-optimization for the allocation.. But it's not clear / documented. At the minimum, the function should be renamed to reflect the inner workings.

The function is a module, and it doesn't do what you'd expect.