Removes direct UUID dep

[jasonkarns]

Summary

UUID is already exposed in the js-sdk-utils package, so we should use it directly.
That will ensure de-duping, as well as automatic tree shaking of the uuid dep (because the utils package imports UUID via ESM).

Before

After

closes #479

[jasonkarns]

Just realized the js-sdk-utils package exposes this same function... So why is it imported directly in optimizely-sdk? The versions now need bumped in both packages so that they'll de-dupe. But the bigger question is: if this is exposed in the utils package, why isn't the utils version being used instead of importing a second copy?

[mjc1283]

Yes, good points. I think we need to remove this dependency from optimizely-sdk, use the version in utils, and update the version in utils to 8.0.0.

[jasonkarns]

PR now updated to remove the direct UUID dep. It's pulled in from js-sdk-utils. Updated tests to stub the correct function.

Also, the js-sdk-utils package is already using the ESM import of UUID, so no version bump is necessary. Indeed, keeping the version at v3* is probably preferable for a while because it increases the chances that downstream consumers can de-dupe UUID itself in their bundles.

[mjc1283]

LGTM, just one comment regarding the object property shorthand syntax.

[jasonkarns]

As a side-note, something I encountered while updating the tests... because all the tests across this suite use the same fake UUID for stubbing the uuid function, when a sinon stub was not properly unstubbed, a stub from one test would poison other subsequent tests. I would strongly suggest using different dummy values between files/suites. That way when a test bleeds over to affect another test, the root cause can be identified much quicker (because the error message will be accurate and not masked by the fact that the UUID is "correct" but for the wrong reason).

Alternatively, since UUID generation is not an expensive operation, I would suggest that the stubbing should be updated to return a different value for each test. That is, remove the static stubbing entirely. Generate a UUID during setup and use that uuid as the stubbed value for the test. That way, a poisoned test run will more quickly expose the root cause; because the UUIDs won't be shared across tests.

[mjc1283]

Everything LGTM, but I overlooked a process issue in my first review. Our packages are published independently, so we can't update optimizely-sdk to use the new stuff in utils until a new version of utils is published. If you could split up the utils changes into its own PR, I can merge that and get a new version published. Sorry for the extra overhead. The fixes here are much appreciated.

[jasonkarns]

It probably got lost in the evolution of this PR, but back here I discovered that the version of UUID being used by the utils package is already ES compliant and tree-shakeable. This PR does contain a tweak to the utils package, but it's purely stylistic. The export of the generateUUID function can be exported directly without being wrapped in another function: https://github.com/optimizely/javascript-sdk/pull/474/files#diff-234d37a781c939ff5538faf0110c211c

However, that change does not need to be released for the optimizely-sdk package to consume the function from utils instead of UUID.

In other words, this PR is entirely focused on optimizely-sdk and can be merged as-is. The change within the utils package can be merged but can remain an unreleased tweak on master.

[mjc1283]

You're quite right - I missed that generateUUID was already exported. Thanks again.