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

Fix @simplewebauthn/server not working in edge environments #455

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

Maronato
Copy link
Contributor

@Maronato Maronato commented Oct 6, 2023

One of @simplewebauthn/server dependencies cbor-x, supports streams, which uses Node's and Deno's stream interface.

Unfortunately, stream is not part of the standard Web APIs that many edge compute runtimes use (e.g. Vercel doesn't support it, and Cloudflare requires a separate node: namespace). Importing it throws errors like these:
Screenshot 2023-10-06 at 13 09 30

Since the only object used from cbor-x is Encoder, the fix is pretty simple. Instead of importing the entire package, we only import its encode file, which is officially exported. This avoids loading the stream.js file that uses stream.

I tested it by patching @simplewebauthn/server directly in my node_modules and it worked without issues, but I haven't attempted to build it locally. I don't see a reason for it not building, though.

@MasterKale
Copy link
Owner

Let's see what CI has to say about this, we'll find out quick if this is works. 🤞

@MasterKale
Copy link
Owner

CI failure:

error: Uncaught "The following specifiers were indicated to be mapped to a package, but were not found:\n * https://deno.land/x/[email protected]/index.js"

You also need to update the path mapping this to an NPM package here:

https://github.com/MasterKale/SimpleWebAuthn/blob/master/packages/server/build_npm.ts#L73

But since we're not importing index.js I don't know hot dnt will handle this 🤔

@Maronato
Copy link
Contributor Author

Maronato commented Oct 6, 2023

Looks like we can use subPath. I'm trying to build it now

@Maronato
Copy link
Contributor Author

Maronato commented Oct 6, 2023

All tests passed with act
Screenshot 2023-10-06 at 14 26 54

@Maronato
Copy link
Contributor Author

Hi @MasterKale!
CI should pass now. Let me know if you need anything else :)

@MasterKale
Copy link
Owner

Thank you for your contribution, @Maronato, this looks good to me :shipit:

@MasterKale MasterKale merged commit 8a5c3be into MasterKale:master Oct 11, 2023
@MasterKale MasterKale added this to the v8.3.2 milestone Oct 11, 2023
@MasterKale
Copy link
Owner

@Maronato This fix is now available in @simplewebauthn/[email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:server @simplewebauthn/server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants