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

Cloudflare worker support #13

Closed
perry-mitchell opened this issue Jan 1, 2023 · 11 comments · Fixed by #25 or #26
Closed

Cloudflare worker support #13

perry-mitchell opened this issue Jan 1, 2023 · 11 comments · Fixed by #25 or #26
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@perry-mitchell
Copy link
Owner

Support ulidx operating in Cloudflare worker contexts, most likely using the monotonic factory feature as active timestamps aren't available. See #5 for more detail.

@perry-mitchell perry-mitchell added the enhancement New feature or request label Jan 1, 2023
@perry-mitchell perry-mitchell added the help wanted Extra attention is needed label Feb 12, 2023
@205g0
Copy link

205g0 commented May 14, 2023

Any news on this?

@205g0
Copy link

205g0 commented May 16, 2023

From the CF docs: Date.now() returns the time of the last I/O.

Why is this not enough? Ofc, the more graular the better, but this should be enough for a good entropy (assuming the second part of the string is long enough).

@perry-mitchell
Copy link
Owner Author

perry-mitchell commented Jun 6, 2023

@205g0 No news, but I don't currently have access to Cloudflare workers to confirm that ulidx works. Once I can confirm that it functions correctly there, at least from a monotonic standpoint, I'll flag this as being compatible.

EDIT: After having a quick look for the ability to run Cloudflare worker scripts locally, I did find this article introducing "Cloudworker". This might be a good addition to the testing setup of ulidx to make sure it runs under such an environment. I'll get around to this soon but would accept a PR for it as well.

@perry-mitchell
Copy link
Owner Author

I'm starting to work on this, with tests being first. Seems there are some libraries that allow emulating worker environments.. once that's done and the tests pass I'll release an update.

@ranile
Copy link
Contributor

ranile commented Jul 6, 2023

This also does not work in Vercel edge functions. The error message being:

✘ [ERROR] Could not resolve "node:crypto"

    node_modules/ulidx/dist/esm/index.js:1:19:
      1 │ import crypto from 'node:crypto';
        ╵                    ~~~~~~~~~~~~~

  The package "node:crypto" wasn't found on the file system but is built into node. Are you trying
  to bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

This happens in a SvelteKit application

@andyjy
Copy link
Contributor

andyjy commented Jul 6, 2023

This also does not work in Vercel edge functions.

@hamza1311 FWIW I am using ulidx v2.0.0 successfully under Vercel Edge functions (thus running on Cloudflare workers).

(With the key note that the timestamp does not increase within the same request due to Cloudflare workers' Date.now() restriction - which doesn't matter for my use-cases.)

✘ [ERROR] Could not resolve "node:crypto"
node_modules/ulidx/dist/esm/index.js:1:19:

Your error suggests you appear be using a version prior to v2 - it references node_modules/ulidx/dist/esm/index.js, whereas v2.0.0 compiles to ulidx/dist/node/.. and ulidx/dist/browser/....

Vercel Edge/CF workers requires using the 'browser' import. This is selected automatically for me with ulidx v2 (using webpack+turbopack config provided by Next.JS) according to the exports configured in ulidx's package.json.
(This was implemented in #20.)

@aaronadamsCA
Copy link
Contributor

FYI @perry-mitchell, Wrangler v3 makes it trivial to run a true worker runtime locally now. It now uses workerd, the same runtime as Cloudflare Workers.

In my experience, this package already seems to work perfectly on Cloudflare Workers.

The only change that would make it easier to use would be to update your package.json exports to rename the "browser" condition to "default" (or add a "worker" condition that points to the same build). As it is now, I had to update my Remix esbuild conditions from ["worker"] to ["worker", "browser"] to get this package loaded; it would be preferable not to tell the build system to load all browser packages just to get this one.

@perry-mitchell
Copy link
Owner Author

perry-mitchell commented Jul 23, 2023

In my experience, this package already seems to work perfectly on Cloudflare Workers.

That's awesome to hear!

@aaronadamsCA That's great advice - I'll add the worker pointer to the exports shortly and will close this issue then. I'd like to add tests locally using such tools so let's see if that makes it into that release or not.

I appreciate all the eyes on this!

@perry-mitchell
Copy link
Owner Author

Reopening as I still want to make some final changes before release.

@perry-mitchell
Copy link
Owner Author

perry-mitchell commented Jul 23, 2023

Going to release this now.
Released 2.1.0.

If anyone has the time to setup tests using wrangler, I think this project would definitely benefit from such a guarantee on stability.

@aaronadamsCA
Copy link
Contributor

Thanks so much! I can confirm this works perfectly; after upgrading to v2.1.0, we can now import { ulid } from "ulidx" in a standard Remix Cloudflare Pages setup with no config changes required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
5 participants