MasterKale / SimpleWebAuthn

WebAuthn, Simplified. A collection of TypeScript-first libraries for simpler WebAuthn integration. Supports modern browsers, Node, Deno, and more.
https://simplewebauthn.dev
MIT License
1.62k stars 137 forks source link

8.3.4 fails to build for edge deployment #471

Closed eglove closed 1 year ago

eglove commented 1 year ago

8.3.4 fails to build for edge deployment.

The recent update from 8.3.3 to 8.3.4 causes issues in edge environments. Using Astro to build a project for Cloudflare Workers I get this error with 8.3.4:

✘ [ERROR] Could not resolve "crypto"

    node_modules/.pnpm/@simplewebauthn+server@8.3.4/node_modules/@simplewebauthn/server/esm/helpers/iso/isoCrypto/getWebCrypto.js:45:43:
      45 │     stubThisImportNodeCrypto: () => import('crypto'),
         ╵                                            ~~~~~~~~

  The package "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.

 error   Could not resolve "crypto"
  File:
    node_modules/.pnpm/@simplewebauthn+server@8.3.4/node_modules/@simplewebauthn/server/esm/helpers/iso/isoCrypto/getWebCrypto.js:45:43
  Code:
    44 |     // dnt-shim-ignore
    > 45 |     stubThisImportNodeCrypto: () => import('crypto'),
         |                                           ^
      46 |     stubThisGlobalThisCrypto: () => globalThis.crypto,
      47 |     // Make it possible to reset the `webCrypto` at the top of the file
      48 |     setCachedCrypto: (newCrypto) => {

node: imports are not available on edge deployments like Cloudflare Workers (Pages), Deno Deploy, and the evil triangle.

Reproduction Steps

Deploy any sample application to an edge environment. Or build a sample project with Astro and @astrojs/cloudflare.

Expected behavior

The node crypto import should be allowed to fail without a program exit and fall back on globalThis. The specific line in the error, "import('crypto')" is asynchronous and throws if it fails to import. I can get a successful local build by just catching it and doing nothing.

import('crypto').catch(error => { /* idk, just don't fail my build! */ }),

SimpleWebAuthn Libraries

    "@simplewebauthn/browser": "^8.3.4",
    "@simplewebauthn/server": "^8.3.4",
    "@simplewebauthn/typescript-types": "^8.3.4",
MasterKale commented 1 year ago

Reproduction Steps

Deploy any sample application to an edge environment. Or build a sample project with Astro and @astrojs/cloudflare.

Hello @eglove, I appreciate your following the issue template but you need to give me a clearer picture of what type of project and runtime you're targeting if I'm going to have confidence that I've solved the problem when I work out a solution. "Any sample application" and "an edge environment" are too vague. A basic reproduction will speed up resolution time too.

eglove commented 1 year ago

I figured the error was obvious enough. I think your previous flow of trying the import first was the right way to go.

I don't really have time for a PR this weekend. If you need something clearer, other than my silly quick fix, it'll have to wait.

I can give you the repo for this, but it won't do you any good.

The 8.3.3 to 8.3.4 change to default to globalThis is a breaking change that makes edge deployments (ex. CloudFlare and Deno Deploy) impossible. Without testing exactly, I don't know if it's the object access or a bundler problem.

Don't really have time to test and debug either. Maybe next week. 😩

MasterKale commented 1 year ago

This should be fixed in @simplewebauthn/server@8.3.5.