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

Drop external uuid dependency in favor of native functionality #4929

Closed
vhscom opened this issue May 15, 2022 · 6 comments · Fixed by #5042
Closed

Drop external uuid dependency in favor of native functionality #4929

vhscom opened this issue May 15, 2022 · 6 comments · Fixed by #5042
Milestone

Comments

@vhscom
Copy link

vhscom commented May 15, 2022

Describe the problem

The default demo currently uses a dev dependency for generating random v4 UUIDs inside the hooks file which has been available natively in Node and the Web Crypto API for some time now. In Node crypto.randomUUID() it has been available since v14.17.0. The Web Cryptography API, which appears to support web workers as well, uses the same method name.

Describe the proposed solution

Dropping this dependency now that support for Node 14 has ended seems like a good time to cut the baggage.

Alternatives considered

Leave the dependency in place. It's slower than native methods but probably not noticeable in the demo.

Importance

nice to have

Additional Information

No response

@Conduitry
Copy link
Member

What happens when the app is deployed with an adapter to an environment that doesn't have the Node runtime available?

@ghostdevv
Copy link
Member

Also luke's uuid package isn't exactly slow:

@jrmoynihan
Copy link

jrmoynihan commented May 15, 2022

Nanoid is even smaller and faster, if I recall correctly.

@paperdave
Copy link
Contributor

ive been looking at a few issues, and i think this change should definitely be done after #4934 is merged, since it seems to include the web crypto global, where the example can use the web crypto method.

@vhscom
Copy link
Author

vhscom commented May 16, 2022

What happens when the app is deployed with an adapter to an environment that doesn't have the Node runtime available?

Thanks. I tried a Pages build after I saw your comment and your intuition was right. The build fails with a reference error as crypto import can't be used in the worker as, in the worker context, it's a global. And we can't currently remove the import in the hooks file (despite valid syntax) as crypto isn't available in the global namespace during the kit build.

@paperdave
Copy link
Contributor

Thanks. I tried a Pages build after I saw your comment and your intuition was right. The build fails with a reference error as crypto import can't be used in the worker as, in the worker context, it's a global. And we can't currently remove the import in the hooks file (despite valid syntax) as crypto isn't available in the global namespace during the kit build.

this is an issue i currently face on my own site, but i have this hack in my svelte config

import { webcrypto } from 'crypto';
global.crypto = webcrypto;

(this works because the config is run only in node environments, and during dev and build its all sharing that global scope)

we obviously shouldn't throw this in the demo, but it's fine as a temp fix for other projects.

@Rich-Harris Rich-Harris added this to the 1.0 milestone May 17, 2022
Rich-Harris added a commit that referenced this issue May 23, 2022
* use crypto.randomUUID() instead of @lukeed/uuid - closes #4929

* changeset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants