-
-
Notifications
You must be signed in to change notification settings - Fork 144
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
Add an ESM build to @simplewebauthn/server to support non-Node runtimes #338
Comments
@rakeshpai Argh, I should have known it was too good to be true. #299 laid the ground work for an ESM build by refactoring away all dependency on NodeJS-specific data types or APIs. That PR did not refactor away the fact that the library is still CommonJS, and thus difficult to incorporate into ESM-based environments. The next thing I plan on tackling is adding an ESM build, so that the project can continue to support CommonJS-based Node projects, while also working in Deno, Bun, Cloudflare Workers, etc... No ETA on that effort, but it's on my roadmap. In fact I'll make this issue the one about adding ESM support, and close this out when I finally deliver. |
I've done it, but on another lib 😄 You might be able to pick up some goodies in these PRs CJS to ESM: Deno support: |
@Hexagon You rock, this will save me so much time 🤩 |
@MasterKale given the title of this issue would you prefer another one to also track CJS to ESM for the other packages like |
@simplewebauthn/browser is already ESM with optional UMD builds too. I don't publish a CJS version of it because it never made sense to. I'm assuming that when I undertake the effort of adding an ESM build to @simplewebauthn/server I'll end up doing the same with typescript-types and iso-webcrypto too because server won't work without those being ESM either. |
So our issue (me and nightah) is related to #293. We can fix it locally by changing I think it's prudent for us to discuss this here as it may affect the future of ESM builds for the server portion. But it's your repository and we'll obviously take your guidance on that. I personally feel the pattern more closely reflects an evergreen build environment as it most closely reflects what files should be transpiled by jest in normal conditions. Having to manually add each module which shouldt be transpiled is a rather ugly solution as if you can only do this with a reverse negative lookahead and you have to have all of these in the same ignore pattern. All that being said I'm NOT someone who should be implicitly trusted to be giving accurate advice as I find the whole cjs/esm and general node landscape to be incredibly wild and confusing. EDIT: In summary I feel like if We used the following in trying to figure all of this out (short read, though it doesn't cover jest): https://bnb.im/series/esm-in-node-js/ |
The NodeJS docs are pretty clear about how Node attempts to determine which module system to use: https://nodejs.org/docs/latest/api/packages.html#determining-module-system
@simplewebauthn/browser should satisfy Scenario 2:
And this has been the case for a while, since #237 added Reviewing #293, which was reported two months later in October 2022, I'm not sure why Jest wasn't processing the files. I wonder, @james-d-elliott, if you'd be willing to help create a basic reproduction of a NextJS project that's unable to run tests because @simplewebauthn/browser is still not sufficiently identifying itself as an ESM module. EDIT: Or if not NextJS then a basic reproduction of whatever setup you and @nightah are having trouble running Jest in seemingly because of browser. |
Not that it matters a great deal but I agree that setting the type to module does satisfy that and I suspect that this a jest issue the more I think about it and delve into it:
I have no trouble setting up an example. |
Just to add to @james-d-elliott's comments and clear up my misunderstanding earlier, it seems that Jest by default expects CJS. I think our options here are:
Also just for clarity the example that we're working with isn't a NextJS project, it's Vite project which could also be part of the problem. I think we need to explore getting a minimal reproduction with Vite/Vitest. |
I apologize if I sounded annoyed, I'm not at all. To get on my soapbox a bit, it's things like "CJS vs ESM" that are annoying to deal with, and I don't want to end up chasing down related rabbit holes trying to solve "an issue with SimpleWebAuthn" that is actually another library's (understandable) struggle to adapt to a bifurcated module system. Thankfully everything is moving towards ESM, and I haven't heard of anything (yet) coming along that might try to usurp ESM, so I feel confident in pursuing ESM support for server. I don't want to add CJS support to browser when everything else front end is now ESM, and it'd only be to paper over some issue with an unrelated library that wants to blend CJS and ESM in otherwise ESM front end projects. Thanks for explaining your situation more. Is "updating Jest to better support ESM" something you might actually try to help with? I wish I had that time anymore, it's all I can do sometimes to keep SimpleWebAuthn moving along 🫠 |
@rakeshpai I'm planning on starting this work on ESM support. Is there a CF worker config/setup you'd suggest I test with that'll ensure I'm actually using the library via ESM, so I don't repeat my earlier "success" that somehow got things working with @simplewebauthn/server's current CJS output? |
Note to self: I guess I'll have to add And
And I think I remember this being an issue when I tried using this library in Deno too. Probably from its ESM-first architecture. Lots of work ahead, it seems... |
Not sure of you need it 😅 But here is a proven method of laying out package.json for esm+cjs. Don't forget to include package.json itself, needed by some build tools. |
Would have shared my project, but it has a lot of fluff with TS, Nx, Astro etc that isn't relevant here. That said, let me list out what I think are the relevant parts of my setup.
Let me know if I can help with more clarifications. |
Thank you for that rundown, that's enough for me to revisit my initial attempt and update it accordingly. [build]
command = "npm run build"
[build.upload]
format = "modules" I'll bet it's these toml file properties that I need to include in my wrangler.toml when I try this again to make sure it's trying to run everything as ESM. Mine was the bare minimum |
I'm able to use server's |
I am unable to load simplewebauthn/sever in deno deploy. You can run snippets in deno deploy playground to recreate the problem. From the logs: The following import returns the same errors locally or on deno deploy: The following import runs locally but returns errors on deno deploy: |
@spendres, What errors do you get when using npm specifiers? Esm.sh is black magic and works if it works 😄 |
No, I would rather use npm specificiers.
…On Thu, Jul 6, 2023 at 2:24 PM Hexagon ***@***.***> wrote:
@spendres <https://github.com/spendres>, I'm curious. Do you have
specific reasons for using esm.sh instead of npm specifiers?
—
Reply to this email directly, view it on GitHub
<#338 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEVLYBFQH3XDC642MR2DODXO37GHANCNFSM6AAAAAAUCYTSBE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
No, I would rather use npm specificiers. (sorry for any duplication from
not using reply all initially)
…On Thu, Jul 6, 2023 at 2:24 PM Hexagon ***@***.***> wrote:
@spendres <https://github.com/spendres>, I'm curious. Do you have
specific reasons for using esm.sh instead of npm specifiers?
—
Reply to this email directly, view it on GitHub
<#338 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEVLYBFQH3XDC642MR2DODXO37GHANCNFSM6AAAAAAUCYTSBE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
When using import SimpleWebAuthnServer from "npm:@simplewebauthn/server@7"; // I've tried 7,7.2,7.2.0,7.3.1 Deno reports that module npm:@simplewebauthn/server is not found. When using import SimpleWebAuthnServer from "https://esm.sh/@simplewebauthn/server@7.2.0"; Deno reports that an internal server error occured. (Cypto?) |
Oooh, i was just curious, but yeah, seems like deno deploy doesn't support npm-specifiers at all (yet)... |
Thanks for the trip report @spendres. I've got some time off coming up mid-August that I plan on dedicating some time to finally crack this ESM nut. I'll definitely be testing with Deno as my ideal place to get @simplewebauthn/server running. |
Breadcrumbs from Deno: import { Fido2Lib } from "https://deno.land/x/fido2@$VERSION/dist/main.js"; fido2-lib repo import deploys to Deno and works with no errors. This may serve as an example to update simplewebauthn lerna/nx scripts to export to deno.land/x. |
I was able to deploy version 7.3.1 using docker and so check in with me
when you start to work on this issue. The issue seems to be CBOR detecting
which runtime it's running on to include the proper native cbor-extract
library.
You are very close!
…On Fri, Jul 7, 2023 at 12:19 PM Matthew Miller ***@***.***> wrote:
Thanks for the trip report @spendres <https://github.com/spendres>. I've
got some time off coming up mid-August that I plan on dedicating some time
to finally crack this ESM nut. I'll definitely be testing with Deno as my
ideal place to get ***@***.***/server* running.
—
Reply to this email directly, view it on GitHub
<#338 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEVLYGHP245KMLXJV2GDATXPAZJXANCNFSM6AAAAAAUCYTSBE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
MasterKale, I have bumped cbor-x library to 1.5.3 in a local repo and the regression test pass. This version of cbor-x works on Deno(both simple test and used by Fido2-lib). How can I test an npm deployment as a candidate v7.3.2 of @simplewebauthn/server? |
I'm using mappings in my dnt config but specify the "^1.2.6" style of package version in the generated package.json. This project has had people ask for me to unpin when I tried pinning dependencies in the past which is why I'm keeping it for Node projects. I just wanted to make sure that Deno projects had some way of updating their project's dependencies without me having to cut a new release every time on of SimpleWebAuthn's dependencies get updated. |
see: (https://deno.land/manual@v1.36.1/basics/import_maps#overriding-imports) My understanding is that deno consumers of SimpleWebAuthn would be able to override a version by placing an import_map.json(or deno.json) file at the root of 'their' project, that would override the versions that you specify in package.json – locked down from your automated testing and in the "publish to npm and deno.land" portions of your release. As an example, let's say that you use ^1.0.0 for dependency A, then publish the package to npm and deno.land. When an application imports SimpleWebAuthn, they would use https://deno.land/x/@SimpleWebAuthn/server@7.4.1/index.ts which would use ^1.0.0 for dependency A. If a patch to dependency A comes out, and downstream users want to update to ^1.0.1 of A, then they would create a version of import_maps.json in their path where they invoke deno(or pass in as command line arg), that would have an entry for A, like:
Deno would detect the local import_map.json(or deno.json) file and look for 'import {X} from A' while loading and substitute A with the URL for A from the import mapping above. |
The earlier post: This version does not deploy on deno since they don't support npm:specifiers. See next email.
import type { |
Μy understanding was correct. I used the deno.lock file to find the npm:specifiers. I kept running:
until there were no more npm:imports. The following import works on deno deploy: I am not sure why, but there are three versions of npm:debug and two versions of npm:types/debug in the release. Here is the deno.jsonc I use in the same directory as the server.ts that loads the SimpleWebAuthnServer import above:
|
This morning I ended up manually finding out the long-form esm.sh URL for each of the packages still using As a matter of fact CI just passed for the first time after refactoring almost the entire monorepo https://github.com/MasterKale/SimpleWebAuthn/actions/runs/5907029108 👀 |
Note to self: I'm so far off the Lerna reservation by moving two of three packages to using dnt for builds. Lerna's "algorithm" or detecting build order doesn't know anymore to build typescript-types before browser or server, and it can't influence the version number specified for This PR will also migrate the project to using npm's workspaces, which means Lerna's primary value now is its The biggest issue right now is that the dnt build does an For example, here's what happens during a
|
Note to Matt: |
Is that something I can do before publishing anything so I know the name is available? |
You have published... to github.com. It may be the 8.0.0"-alpha-" version, but SimpleWebAuthn is open source, in a public repository, and they can verify that you own the repo. That should be enough for you to claim the url. Since, we can deploy on dash.deno.com now, they should be able to steer you over the last hump after DNT. One step I know is that you'll need to create a github webhook to fully automate publication of future releases to deno.land/x/SimpleWebAuthn –triggered when you create a tag on your designated branch. You can change the "branch and tag" after you merge back to main, or wait to complete the setup for the webhook until after you merge back to main. It looks as though the creation and verification of the webhook file is enough for them to verify that you control the repo – at least that is what I infer from reading their site. |
Okay, things are in a pretty decent place right now. The one thing I need to work through now is a new publishing routine since dnt builds and then wants to And NPM workspaces are too dumb to know how to replace Honestly the only thing I'd be keeping Lerna around for at this point is it's |
Nice! I was able to verify that these shorter import urls don't break deno deploy using the head of your branch:
|
Something to consider:
|
I spy with my little eye... https://deno.land/x/simplewebauthn@v8.0.0-alpha.0 👀 |
The plan is I'll put this release through its paces, and so long as things are looking good for server in Node, Deno, and CF Workers (I really want it to work in this last one for some reason, even though I don't have any immediate need to build anything with them...), and so long as browser doesn't appear to be negatively impacted by the tweaks to the publishing process, I'll finalize #425 as the PR that finally closes this ticket... I'd also appreciate your help in confirming these things work in the other environments:
Thanks for your help thus far, everyone - the end is in sight! 🙇 |
Yikes! Alpha release works on deno deploy using the following import:
What do you think about adding symlinks or root level (re imports/exports) to shorten URLs to:
|
I'm not against this idea, I'm thinking an
Let me consider that a bit more 🤔 |
Having .ts files flailing around in the root of the project feels gross to me, so I'm going to keep them in a deno/ folder:
The imports are a bit long when the version is something nuts like test_deno.ts import {
generateAuthenticationOptions,
} from 'https://deno.land/x/simplewebauthn@v8.0.0-alpha.1/deno/server.ts';
import {
generateChallenge,
} from 'https://deno.land/x/simplewebauthn@v8.0.0-alpha.1/deno/server/helpers.ts';
console.log(await generateAuthenticationOptions());
console.log(await generateChallenge()); $> deno run test_deno.ts
{
challenge: "df7BtyMwIEeM08ea1vUKnhiIKpRxxOt96T_SS0hMots",
allowCredentials: undefined,
timeout: 60000,
userVerification: "preferred",
extensions: undefined,
rpId: undefined
}
Uint8Array(32) [
216, 79, 49, 33, 40, 33, 5, 19,
188, 211, 157, 226, 86, 167, 97, 38,
4, 250, 24, 146, 141, 68, 242, 1,
62, 140, 90, 141, 29, 3, 141, 11
] 🎉 |
That looks clean.
…On Mon, Aug 21, 2023 at 12:41 PM Matthew Miller ***@***.***> wrote:
What do you think about adding symlinks or root level (re imports/exports)
to shorten URLs to:
***@***.***/browser.ts";
***@***.***/server.ts";
***@***.***/browser/helpers.ts";
***@***.***/server/helpers.ts";
***@***.***/types.d.ts";
Having *.ts* files flailing around in the root of the project feels gross
to me, so I'm going to keep them in a *deno/* folder:
***@***.***/deno/browser.ts"
***@***.***/deno/server.ts"
***@***.***/deno/browser/helpers.ts"
***@***.***/deno/server/helpers.ts"
***@***.***/deno/types.d.ts"
The imports are a bit long when the version is something nuts like
v8.0.0-alpha.1 but I think they'll be fine when this officially gets
released (and besides, deno fmt is taking care of stuff like this for
you, right? 😂)
*test_deno.ts*
import {
generateAuthenticationOptions,} from ***@***.***/deno/server.ts';import {
generateChallenge,} from ***@***.***/deno/server/helpers.ts';
console.log(await generateAuthenticationOptions());
console.log(await generateChallenge());
$> deno run test_deno.ts
{
challenge: "df7BtyMwIEeM08ea1vUKnhiIKpRxxOt96T_SS0hMots",
allowCredentials: undefined,
timeout: 60000,
userVerification: "preferred",
extensions: undefined,
rpId: undefined
}
Uint8Array(32) [
216, 79, 49, 33, 40, 33, 5, 19,
188, 211, 157, 226, 86, 167, 97, 38,
4, 250, 24, 146, 141, 68, 242, 1,
62, 140, 90, 141, 29, 3, 141, 11
]
🎉
—
Reply to this email directly, view it on GitHub
<#338 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEVLYFO3OJYB4PDR6FMNLLXWOFSZANCNFSM6AAAAAAUCYTSBE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This isn't a completed integration, but the 8.0.0-alpha.0 version deploys on cloudflare workers using npm:
|
Great, that's the same thing I was seeing locally using wrangler and And |
@spendres (pulling in your comment on the now-closed #366) The thought crossed my mind but I think it might be confusing to call the root folder with the import shortcuts esm/ because "how does that distinguish it from the ESM build that's located in packages/server/npm/ directory if you build @simplewebauthn/server with dnt?" Naming things is hard 🙃 |
I would think that the esm directory created by the build_npm tool would be functional, but it won't be until the deno.jsonc file contains the esm import mappings. As it is, the npm/esm directory is unusable – unless I'm missing something. Once the mappings render a working npm/esm, then its contents should functionally be a replica of the current deno directory. But its only "deno" characteristic is that it was created with dnt. The current "deno" name might prevent adoption simply because users are looking for esm, but may not be running on deno. The ESM standard is separate from the deno runtime. For example, #366 requested server-esm... not server-deno. Naming things is definitely hard 😬 This isn't reason to delay release... but maybe something to track. |
The point I'm trying to make here is that there will be three possible ways to use SimpleWebAuthn:
For the first two, the package published to NPM contains both a CJS version and ESM version. But they're only for use in Node and Node-compatible runtimes (like Bun is shaping out to be) For the last, I'm only aware of Deno that has such a module import system as "point to a URL." Yes, modules that work with Deno are ESM code because that's all it supports. But I'm not yet aware of other runtimes that support Deno's module import mechanism that might have developers confused that "SimpleWebAuthn only supports Deno with the imports in deno/."
Of all the JS server runtimes I've been tracking, Node, CloudFlare Workers, and Bun seem to all support installing packages from NPM which is why it was important for me that I continue to publish packages there that support both CJS and ESM use. Deno really is the most unique because it has nothing to do with NPM, so I think that kinda shifted my perspective on what exact use cases I'm trying to enable with #425. Are there any other such runtimes as Deno right now? Any coming out soon?
Edit: this morning I realized that the code in server/src/ all points to a deps.ts file that imports from full URLs. None of these imports will be of use to anything other than Deno projects, so the deno/*.ts files are as Deno-centric as the contents of server/src/ now is. This has convinced me that deno/ is the right name for the folder containing those .ts files. |
It’s good to know your thinking on this.
…On Tue, Aug 22, 2023 at 12:04 AM Matthew Miller ***@***.***> wrote:
The point I'm trying to make here is that there will be three possible
ways to use SimpleWebAuthn:
1. Node in "traditional" CJS mode
2. Node in ESM mode
3. Deno
For the first two, the package published to NPM contains both a CJS
version and ESM version. But they're only for use in Node and
Node-compatible runtimes (like Bun is shaping out to be)
For the last, I'm only aware of Deno that has such a module import system
as "point to a URL." Yes, modules that work with Deno are ESM code because
that's all it supports. But I'm not yet aware of other runtimes that
support Deno's module import mechanism that might have developers confused
that "SimpleWebAuthn only supports Deno with the imports in *deno/*."
Of all the JS server runtimes I've been tracking Node, CloudFlare Workers,
and Bun seem to all support installing packages from NPM, which is why it
was important to publish packages to it that contain both CJS and ESM. Deno
really is the most unique because it has nothing to do with NPM. Are there
any other such runtimes as Deno right now? Any coming out soon?
I'm probably overthinking this 🤔
—
Reply to this email directly, view it on GitHub
<#338 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEVLYD6TUJZ4K23234PBYLXWQVVVANCNFSM6AAAAAAUCYTSBE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
PR #425 has been merged and released as SimpleWebAuthn v8.0.0 in the usual places:
And of course now on Deno: https://deno.land/x/simplewebauthn@v8.0.0 Thank you all very much for your support, especially @spendres and @Hexagon! This has been a long time coming and I'm happy that I'm able to finally close this issue 😌 |
Time to roll it all back! 😂 https://deno.com/blog/npm-on-deno-deploy (Just kidding, this was all worth it to natively support Deno and get the project in a state that should better survive the introduction of future runtimes |
OMG! Hilarious...
…On Wed, Sep 6, 2023 at 2:17 PM Matthew Miller ***@***.***> wrote:
Time to roll it all back! 😂
https://deno.com/blog/npm-on-deno-deploy
(Just kidding, this was all worth it to natively support Deno *and* get
the project in a state that should better survive the introduction of
future runtimes
|
FYI: Here is a simple Passkeys/Deno/KV implementation using
SimpleWebAuthn on deno.land using ^8.1+:
https://passkeys.deno.dev
…On Wed, Sep 6, 2023 at 2:29 PM Steven Endres ***@***.***> wrote:
OMG! Hilarious...
On Wed, Sep 6, 2023 at 2:17 PM Matthew Miller ***@***.***>
wrote:
> Time to roll it all back! 😂
>
> https://deno.com/blog/npm-on-deno-deploy
>
> (Just kidding, this was all worth it to natively support Deno *and* get
> the project in a state that should better survive the introduction of
> future runtimes
|
I know I'm probably taking this lib to environments it wasn't designed for, but I thought I'd try my luck anyway.
I'm working with Miniflare, which is Cloudflare's local dev environment thing. When I invoke
generateRegistrationOptions
, I get the following error:Error: Dynamic require of "stream" is not supported
All the stack frames are in my code, because the code is minified before it is passed into miniflare, so it's very hard to get to the root of the problem. However, from what I could find out from the minified code was that the issue is probably not with the SimpleWebAuthn code itself, but with the way it is packaged.
Here's a screenshot of the offending code, if it's useful.
My suspicion is that this could be fixed by packing up the server code as ESM, maybe? Right now, the
@simplewebauthn/server:/server/dist/*.js
is using node's require syntax, which requires me to fall back to miniflare's legacy handling of modules, which I'd rather not do.Is there a chance that you could package up the server in an ESM format for consumption in modern ESM environments?
The text was updated successfully, but these errors were encountered: