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

[RFC] Permit bundling in a WinterCG environment #109

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sujayakar
Copy link

I was trying to get OpenAuth working with the Convex runtime environment, which is close to WinterCG compatibility and doesn't implement Node APIs.

Bundling authorizer.ts from examples/authorizer/custom-frontend/auth fails for this target since it includes Node APIs in its import tree:

$ bun run release && esbuild examples/authorizer/custom-frontend/auth/authorizer.ts --bundle --format=esm --target=esnext --platform=browser --metafile=/tmp/meta.json --outfile=/tmp/out.js

✘ [ERROR] Could not resolve "fs"

    packages/openauth/dist/esm/storage/memory.js:3:41:
      3 │ import { existsSync, readFileSync } from "fs";
        ╵                                          ~~~~

✘ [ERROR] Could not resolve "crypto"

    packages/openauth/dist/esm/random.js:2:32:
      2 │ import { timingSafeEqual } from "crypto";
        ╵                                 ~~~~~~~~

✘ [ERROR] Could not resolve "fs/promises"

    packages/openauth/dist/esm/storage/memory.js:4:26:
      4 │ import { writeFile } from "fs/promises";
        ╵                           ~~~~~~~~~~~~~

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

    node_modules/hono/dist/adapter/aws-lambda/handler.js:2:19:
      2 │ import crypto from "node:crypto";
        ╵                    ~~~~~~~~~~~~~

Poking through the code, I think these Node dependencies fall into one of three categories:

  1. The memory storage depending on node:fs for the persist flag: I set this to use dynamic feature detection, where setting persist = true will now fail at runtime if the Node APIs aren't available.
  2. Timing safe comparisons: I just changed this to regular string comparison, but we'd need to find a WinterCG compatible replacement to actually land this.
  3. The reexport of import { handle as awsHandle }from "hono/aws-lambda". Perhaps we could move this into another entry point so non-AWS importers can tree shake it away.

With these changes, I can get the esbuild command working, and the bundle looks good!

Let me know if y'all think this is a direction you'd want to take OpenAuth. With Bun, Deno, and Cloudflare Workers all pursuing Node API compatibility these days, I think it's also pretty reasonable to just target Node.

@bachiitter
Copy link

bachiitter commented Dec 24, 2024

@sujayakar may be give deno @std/crypto a try because that is compatible with most environments

@sujayakar
Copy link
Author

@sujayakar may be give deno @std/crypto a try because that is compatible with most environments

unfortunately, it's not supported in the convex runtime environment (since it's not part of wintercg).

@bachiitter
Copy link

@sujayakar may be give deno @std/crypto a try because that is compatible with most environments

unfortunately, it's not supported in the convex runtime environment (since it's not part of wintercg).

aha didn't knew that my bad

@thdxr
Copy link
Contributor

thdxr commented Dec 26, 2024

can you share again the esbuild command you were running that was including node deps?

something is not getting treeshaken right if the memorystorage stuff was getting included even if you were not using it

@sujayakar
Copy link
Author

can you share again the esbuild command you were running that was including node deps?

something is not getting treeshaken right if the memorystorage stuff was getting included even if you were not using it

I'm running

bun run release && esbuild examples/authorizer/custom-frontend/auth/authorizer.ts --bundle --format=esm --target=esnext --platform=browser --outfile=/tmp/out.js

but it still fails setting --tree-shaking=true:

bun run release && esbuild examples/authorizer/custom-frontend/auth/authorizer.ts --bundle --format=esm --target=esnext --platform=browser --tree-shaking=true --outfile=/tmp/out.js

maybe esbuild isn't able to prove that the imports don't have side effects?

@aryasaatvik
Copy link
Contributor

imo storage adapters should be injected as a part of the config not using env vars #136

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 this pull request may close these issues.

4 participants