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.63k stars 137 forks source link

What to do about `supportsWebAuthn()`... #114

Closed MasterKale closed 3 years ago

MasterKale commented 3 years ago

As highlighted in PR #112 and #113, @simplewebauthn/browser's supportsWebAuthn() method can't actually execute in browsers that don't support WebAuthn, like IE11 and Edge Legacy, because those browsers don't support ES2018 syntax. This negates the reason for its existence.

As I mentioned in PR #113, the path forward is ambiguous to me. It's likely not as simple as updating packages/browser/tsconfig.json to target ES5 instead of ES2018.

Possible options I proposed included:

I'm open to suggestions on how we might solve the problem of building a library that simultaneously needs to sometimes support ES5 while often supporting ES2018.

MasterKale commented 3 years ago

For the record I'm leaning towards option 3️⃣ because it's the easiest to pull off 😂

mariusmarais commented 3 years ago

(Trying to be more constructive the second time around...)

It seems that it's possible to build for different environments using babel, e.g. https://stackoverflow.com/a/52024307 (It is, but I think it's off-target in this case.)

Since you're already using webpack, option 1 shouldn't be that hard, but I haven't attempted it myself. Maybe start with the multiple targets example here https://webpack.js.org/concepts/targets/ and add Babel in ES5 mode in the second target? Add both outputs to the NPM module and users load which they prefer?

Another nice example using webpack-merge, since these configs will almost be identical: https://stackoverflow.com/questions/55826856/how-to-build-for-multiple-targets-with-webpack

akanass commented 3 years ago

@MasterKale The best solution is to stop using Webpack because this tool is old school and to have a better build system with multiple targets, the right tool to use is rollup.

I use it in my projects and even with the one that supports this library, I have no compilation problems and I output files in MJS for browsers supporting imports of ESM modules or even SystemJS compatible files with an import modules inside.

When we have multiple bundle, we need to have different entries point inside package.json to be used in other bundle system like main, browser, jsnext:main, module, ....

akanass commented 3 years ago

But once again, why compile this library in ES5 when the browsers that support WebAuthn all support ES2018 compilation.

Older browsers don't support WebAuthn so I don't see the point of all of this.

mariusmarais commented 3 years ago

@akanass I assume the underlying issue is that this non-ES5 module is blowing up the entire app. For the app WebAuthn is optional and only available in newer browsers. But the pure existence of the option is breaking the app in the older browsers. Since the app must work in older browsers because of reasons, the WebAuthn functionality cannot be used by anyone. (This is putting a lot of words in @Moumouls 's mouth, I hope the gist is correct.)

Objectively the code as written has a bug: it cannot possibly work as originally intended. To fix it, the detection code must be ES5 compatible, because it can +- never fail to detect, only blow up. The easiest thing to do is to make everything ES5, but ... reasons.

After successful detection, we know newer code can be loaded, but splitting on that level with dynamic imports is very complex and possibly out of scope for this module. The midway is to build both ES5 and newer versions, but that burden might be too high for this package, especially if it would require a build system switch (regardless of which is better or newer).

In short, the situation sucks. (At work I would have said "nuanced" 😄 ) So no-one is crazy, they just have different goals and @MasterKale wants to help without ruining his week.

akanass commented 3 years ago

@mariusmarais @MasterKale I totally agree with both of you and the best solution is too have multiple bundle in this library or just indicated to user who wants to use it inside their own project in ES5 that they have to transpile it.

You can see an example here how to transpile ES6 to ES5 with Webpack and babel https://medium.com/@sivaraj-v/basic-webpack-4-and-es5-to-es6-transpiler-using-babel-dc66e72c86c6

This library should not be in ES5 natively because people who are using dynamic import will be badly impacted and it should not be the case.

akanass commented 3 years ago

Here a build system with webpack and multiple targets output

akanass commented 3 years ago

With multiple bundles, the main entry of package.json will be the ES2018 version, like this all the people who have a good build system or who want to use dynamic import or module in browser will be able to do it and the browser entry of package.json will be the ES5 version, like this people who are using Webpack to build a browser version will have this version in their own build and they have to set the right option in their webpack.config to take it

Moumouls commented 3 years ago

@mariusmarais you are right the situation sucks.

So: With ES5 9kb (without GZIP and tree shaking ) Without ES5 5kb (without GZIP and tree shaking )

So final diff is 4kb heavier...

Here a 4ko diff in 2021 looks like to me a good tradeoff to avoid over head/surprise for developers that use the lib and avoid to some developers to have their app (especially the login page) starting crashing on old browser since. Yes (sadly) in 2021 Legacy Edge and IE11 still seems to be used. The lib will work by default without over head or special attention of the developer.

In other hand, if developers want's to optimize their build there are also free to re compile the lib, dive into the compilation process to get rid off of the 4ko and add some additional configs/systems to load it correctly in their projects.

akanass commented 3 years ago

I don't agree on it because I have the ES2018 version in my project and I can build it in SYSTEMJS, COMMONJS, ESM, ES5 whatever and all is working without any problems because I am using the right tool to do it.

We have not to provide an old school version to fix problem or say to developers you have to rebuild the lib.

Developers should know how to build a ES2018 to ES5 without problem to include it in their own project and not the reverse.

All modern framework are using ES2018 as standard and provide polyfills if needed or just explain that you have to compile this version in ES5 if you need it.

ES5 as default is not the solution. Having multiple version in the build is one of the best or just make developers responsible and they have to do the job by their own

akanass commented 3 years ago

If you add a polyfill in your project to support ES2108 syntax all will work if you don't want to make the setup to compile in ES5 in your application.

Include the polyfill in your bundle process and all will be done

Moumouls commented 3 years ago

So here since this issue is critical for my company use case and also for further Parse SDK implementation (Parse community). i opted out for a re published package under an alt org https://www.npmjs.com/search?q=%40simplewebauthn-alt.

I really don't like re publishing projects like this one, well constructed. 🙁 I can't let the apps crash on the old browser any longer...

@MasterKale i'll add some info on the README to indicate the reason of the re publish.

Note: the downgrade to ES5 works correctly, tested with browser stack and legacy edge.

akanass commented 3 years ago

@Moumouls you said that you're using Next.js application and the problem occurred with the spread operator but in the documentation of Next.js I can see they are supporting this operator with IE11 and they are giving some advanced configuration to allow babel to add more plugins and solve your problem I guess

MasterKale commented 3 years ago

So here since this issue is critical for my company use case and also for further Parse SDK implementation (Parse community). i opted out for a re published package under an alt org https://www.npmjs.com/search?q=%40simplewebauthn-alt.

I can't say I'm a fan of this approach - everything on all of those "-alt" listings links back to here. Can you not pull in @simplewebauthn/browser into your codebase temporarily while I work on an official solution to this problem?

Moumouls commented 3 years ago

@akanass Next.JS support IE11 for code compiled by Next (in our case React Components). Next ensure that compiled react components will work in IE11, Next does not guarantee/re compile dependencies (just perform some tree shaking). On all our packages that we use only simplewebauthn makes our app crashing.

@MasterKale sadly my team already use the re published package and we have also have an ETA scheduled for our customers tomorrow. We cannot remove the feature temporarily since our customers already use it and we can't keep an app crashing on login page on old browsers.

while I work on an official solution to this problem?

May be a temporary solution could be to change the compilation target to es5 while waiting the final solution (multi export mentioned by @akanass )

Then when the browser package will be patched, i will for sure delete the re published packages, and switch back to the original lib. 🙂

Here I just want to help, on our side we have Q/A teams waiting for an ETA, and the fastest way for us is just to re publish the package.

akanass commented 3 years ago

@Moumouls if they provide advanced settings to add babel loader it's for something and you can override the config to indicate which package you want to compile or not. Instead of exclude node_modules like all webpack.config does you can indicates that you want to compile simplewebauthn too.

When I am using rollup and I import browser functions, they are compiled inside my own files in the target that I want and I don't have problem even in old browser.

Moumouls commented 3 years ago

thanks @akanass for the tips but on our side the release is already on the road and the fix works without over head or additional config. Now here i think it should be interesting to focus on how to support multi export here to switch back asap and avoid surprise for developers that do not dive into each package configs 🙂

MasterKale commented 3 years ago

The good news is I found some time to follow that article @akanass posted. I have a branch sitting on my laptop that builds the lib in a few different configurations (basically everything except CJS because it's not relevant in front end projects)! 🎉

The only blocker right now is finding a version injection plugin that will let me use multiline comment blocks to avoid issues with single line comments breaking some build configurations that have come up in the past...

@akanass: how would someone using a newer version of @simplewebauthn/browser, built similarly to that article you posted, "choose" the desired version of the bundled or unbundled files to use? And do .d.at files map to bundled and unbundled files too? So types would be available no matter which set of files a consuming project chose to reference?

MasterKale commented 3 years ago

@Moumouls I just pushed up a fix/issue-114-browser-multi-bundle branch here. Would you be willing to test out the output from the new @simplewebauthn/browser build process? You can npm run build:browser from the root folder to build it, and see the new build artifacts in packages/browser/dist/:

Screen Shot 2021-04-02 at 8 25 20 AM

In the meantime I'll be trying to find time and figure out the best way to test older browsers myself. I figured I'd put the work up here in case you were interested in being a bit of a guinea pig since this impacts you the most 😇

MasterKale commented 3 years ago

And here's the new package.json - @akanass does this look right?

"main": "dist/esm/index.js",
"exports": {
  "import": "./dist/esm/index.js"
},
"types": "./dist/types/index.d.ts"

The way I see it the bundled UMD will be more suitable for linking to this package from something like unpkg, when you want to manually pull in a single JS file and host it the "old-fashioned" way, within a <script>.

akanass commented 3 years ago

@MasterKale I'm not sure it's OK with the package.json and even with the bundle.

I've checked your branch and the problem is that you only have an ESM version with ES2018 as target.

You have to build a version in ES5 and another one in ES2018.

For ES5 version, your tsconfig.json should have the properties:

{
  "compilerOptions": {
    "target": "es5",
    "module": "commonjs",
  },
  "lib": [
    "es5",
    "dom"
  ],
}

For ES2018 version, your tsconfig.json should have the properties:

{
  "compilerOptions": {
    "target": "es2018",
    "module": "es2020",
  },
  "lib": [
    "es2018",
    "dom"
  ],
}

And after in your package.json you should have:

{
  "main": "./dist/esm2018/index.js",
  "browser": "./dist/esm5/index.js",
  "types": "./dist/types/index.d.ts"
}

We want to have the latest version as default version that's why we put it in the main property and we create the browser entry that can be used by Webpack like explain here

The exports has not to be used here because we have only ESM build so they can be both imported by import or require keywords.

During the development, may import the version used for that in ES5, that in ES2018 or that in TS, what counts is how the elements are compiled inside the final project and I think the configuration like this will work.

I do not think the bundle versions are needed apart if you really want to provide the possibilities of being able to import directly into a script tag but I'm not a fan of what is done in the example because the bundle included far too much of something from babel.

It would be necessary to take either the sources already compiled in ES5, for the UMD version, and the ES2018 for the ESM version or use rollup to make the complete process of TS to JS then the bundle version but as is done here this is not at all optimal.

I think these bundle versions complicates the thing if you do not control rollup so the ES5 and ES2018 versions are largely sufficient in my opinion.

MasterKale commented 3 years ago

You have to build a version in ES5 and another one in ES2018.

My understanding is that Babel is responsible for transpiling down to ES5-compatible code based on the value of "targets" passed to @babel-env in .babelrc.js

akanass commented 3 years ago

@MasterKale you're right but this value is used only for the esmBundled env which uses rollup after.

But for your dist/esm you are not using it as we can see on your script here your are using the esmUnbundled env which means you are only compiling TS to JS with the default tsconfig.json so you will only have the ES2018 version and you can check the code if you still have or not the spread operator inside which is the reason of the browser crash.

If the code is transpiling down to ES5-compatible as you think and maybe it's the case, and if we only have this version, it's not good as well because we want to have both versions

akanass commented 3 years ago

Because we only want ES5 and ES2018 versions the easy way will be to use TSC to compile with one dedicated tsconfig file for both version instead of using babel

We don't need the bundled versions at all so make it simple and all will work.

And after if you really want to have the bundled version you can use the ES5 version as entry point for rollup and all will be perfect.

akanass commented 3 years ago

The example link I provided it was to explain the global scope but the easiest way to implement the thing is to use the Typescript compiler with two configuration files and the turn is played.

MasterKale commented 3 years ago

In my mind we want "unbundled ESM targeting ES2018 (so that consumers can let their build tools handle polyfilling and transpiling down to ES5)", and "bundled UMD targeting ES5". So thinking about it some more I probably don't need to have Rollup produce anything ESM, just bundled UMD which will lean on Babel for targeting ES5.

Because we only want ES5 and ES2018 versions the easy way will be to use TSC to compile with one dedicated tsconfig file for both version instead of using babel

We don't need the bundled versions at all so make it simple and all will work.

I understand how you're proposing I simplify the build to just two executions of tsc, but that won't produce anything bundled, so it'd be impossible to use a service like unpkg to download a single JS file for easy copy/paste into an HTML file (which is currently supported, it's how I pull in Browser in example/)

akanass commented 3 years ago

@MasterKale it's not the case because @Moumouls is using the compiler of Next.js to bundle his application and inside the code you have the import of your library which means you have to provide an ES5 version too.

People won't only use script tag for the ES5 version. If they want to use your library inside their own script and they don't transpile it during their build process, an ES5 version has to be provided as well.

All browser librairies provide a bundle and a module version compatible or not with old browser. If you want to have the compatibilty, the module version has to be in both versions

akanass commented 3 years ago

@MasterKale

I understand how you're proposing I simplify the build to just two executions of tsc, but that won't produce anything bundled, so it'd be impossible to use a service like unpkg to download a single JS file for easy copy/paste into an HTML file (which is currently supported, it's how I pull in Browser in example/)

yes you can have the bundled version with rollup after compiling with TSC, you will use the output as entry point to build your bundled version and you won't have all boilerplate of babel because the entry point will be the ES5 version made by TSC

MasterKale commented 3 years ago

All browser librairies provide a bundle and a module version compatible or not with old browser. If you want to have the compatibilty, the module version has to be in both versions

Okay, so then I need how many outputs...

  1. ESM unbundled, targeting ES2018
  2. ESM bundled, targeting ES2018
  3. ESM unbundled, targeting ES5
  4. ESM bundled, targeting ES5
  5. UMD bundled, targeting ES5

Is that all the permutations I need? I'm having a hard time understanding when I do or don't need to bundle.

akanass commented 3 years ago

You need:

  1. ESM unbundled, targeting ES2018 made with TSC and dedicated tsconfig.es2018.json configuration file:
{
  "compilerOptions": {
    "target": "es2018",
    "module": "es2020",
    "declaration": true,
    "declarationDir": "./types"
  },
  "lib": [
    "es2018",
    "dom"
  ],
}
  1. ESM unbundled, targeting ES5 made with TSC and dedicated tsconfig.es5.json configuration file:
{
  "compilerOptions": {
    "target": "es5",
    "module": "commonjs",
    "declaration": false
  },
  "lib": [
    "es5",
    "dom"
  ],
}
  1. ESM bundled, targeting ES5 with the same tsconfig.es5.json configuration file than previous one, made with rollup and all those plugins @rollup/plugin-typescript, @rollup/plugin-commonjs, @rollup/plugin-node-resolve, rollup-plugin-terser, @rollup/plugin-json are required

The rollup config for the bundled versions will be:

// rollup plugins
import typescript from '@rollup/plugin-typescript';
import commonjs from '@rollup/plugin-commonjs';
import nodeResolve from '@rollup/plugin-node-resolve';
import { terser } from 'rollup-plugin-terser';
import json from '@rollup/plugin-json';
import versionInjector from 'rollup-plugin-version-injector';

export default {
  input: 'src/index.ts',
  output: [
    {
      file: 'dist/bundles/bundle.umd.min.js',
      format: 'umd',
      name: 'SimpleWebAuthnBrowser',
      sourcemap: true,
      plugins: [terser()]
    },
  ],
  plugins: [
    typescript({tsconfig: './path/to/tsconfig.es5.json'}),
    commonjs({extensions: ['.js', '.ts']}),
    nodeResolve({mainFields: ['jsnext:main', 'module', 'main']}),
    json(),
    versionInjector({
     injectInComments: {
       fileRegexp: /\.(js)$/,
       // [@simplewebauthn/browser]  Version: 2.1.0 - Saturday, February 6th, 2021, 4:10:31 PM
       tag: '[@simplewebauthn/browser]  Version: {version} - {date}',
       dateFormat: 'dddd, mmmm dS, yyyy, h:MM:ss TT',
    },
  }),
  ]
}

Your package.json will have the entries:

{
  "main": "./dist/esm2018/index.js",
  "browser": "./dist/esm5/index.js",
  "types": "./dist/types/index.d.ts"
}

With all of this, all should work correctly and we'll be able to test it and ask @Moumouls to build his application with this version.

MasterKale commented 3 years ago

Thank you for breaking all of that down. SimpleWebAuthn is (obviously) my first foray into maintaining a browser library and I'm always learning something new.

{
  "main": "./dist/esm2018/index.js",
  "browser": "./dist/esm5/index.js",
  "types": "./dist/types/index.d.ts"
}

Seeing examples like this helps me learn the quickest, so thank you for this too.

Am I correct in understanding that "main" and "browser" point to the unbundled builds? I have a couple of questions if that's the case:

  1. What would seek to pull in the bundled code instead?
  2. How would someone like @Moumouls reference "browser" since they need to support ES5?
akanass commented 3 years ago

@MasterKale yes you are right, main and browser point to to the unbundled build because they are used in consumer's project build system.

For your questions:

1 - If you're using typescript in your project, you will import from types definition when you develop. If you're using javascript instead of, you will import from main definition. When you will build your project, your builder will take one of main or browser depending of the configuration you provide and I'll explain more in the second point.

2 - When you build your application with webpack you have to specify the target and by default it's web so the resolve main fields for this target are browser then module then main as explain here.

In the case of @Moumouls who is using Next.js and webpack the compilation will take the browser version of your library because it's the first in the declaration order for the web target in the default configuration. If he changed the default configuration, he will have to put the browser value first to be sure that it is the one that is taken into account as the default value.

For people who use rollup as a build system, they will have to specify in the node-resolve plugin the fields to be taken into account as I did in the configuration I gave you for your own bundle and they will add the value browser also first as explain here

Like this all will work and build system will understand what to take to build correctly.

MasterKale commented 3 years ago

@akanass Okay, I found some time to go through everything and adjust the build process to use tsc for everything. So far so good, and I'm glad I'm able to remove Babel to keep things simple.

What I still can't make sense of, though, is why I'm bothering to generate four ES5 bundles in both ESM and UMD flavors. I can understand creating one of the bundles, the minified UMD build, for including the library via a service like Unpkg. However nothing else makes sense to me given:

In my mind things simplify even further and Rollup only becomes responsible for outputting a minified ES5 UMD bundle via tsconfig.es5.json.

Edit: Why do I use "module": "commonjs" in tsconfig.es5.json? Is it easier to bundle CommonJS modules for older browsers than if I used, say, "module": "es6" instead? CommonJS is a Node thing so it doesn't seem applicable to this browser library.

MasterKale commented 3 years ago

Ugh, and if I try to use "module": "commonjs" within tsconfig.es5.json, Rollup doesn't seem to go beyond processing src/index.ts. This is the content of dist/bundles/bundle.umd.min.js:

// [@simplewebauthn/browser]  Version: 2.1.0 - Sunday, April 4th, 2021, 10:11:42 PM
!function(t){"function"==typeof define&&define.amd?define(t):t()}((function(){"use strict";Object.defineProperty(exports,"__esModule",{value:!0});var t=require("tslib"),e=t.__importDefault(require("./methods/startAttestation")),r=t.__importDefault(require("./methods/startAssertion")),s=t.__importDefault(require("./helpers/supportsWebauthn"));exports.default={startAttestation:e.default,startAssertion:r.default,supportsWebauthn:s.default}}));
//# sourceMappingURL=bundle.umd.min.js.map

A screenshot shows it stops at actually importing anything imported into this file: Screen Shot 2021-04-04 at 10 15 08 PM

The @rollup/plugin-typescript plugin also complains that I'm not using "module": "esnext":

(!) Plugin typescript: @rollup/plugin-typescript: Rollup requires that TypeScript produces ES Modules. Unfortunately your configuration specifies a "module" other than "esnext". Unless you know what you're doing, please change "module" to "esnext" in the target tsconfig.json file or plugin options.

If I change to "module": "esnext" in tsconfig.es5.json to appease the warning then the rest of the code gets included in the bundle, but for some reason the TypeScript copyright notice gets included too! 😖

Screen Shot 2021-04-04 at 10 18 02 PM

What is going on here...

akanass commented 3 years ago

@MasterKale my bad I missed something in my rollup.config so I have adapted my previous comment

I've changed the config and put the right one inside commonjs plugin like explain here

For a target in ES5, the module has to be set with commonjs as explain in the officiel typescript documentation here that's why I've changed the rollup.config as well.

For old browser with target: es5, the ESM syntax can't be understood so the commonjs wrapper has to be used especially because we're using the same config for TSC and rollup so let module: commonjs and all will work normally with the updates made.

For the bundle you're right too, you only need the UMD minified version and it's changed too in the rollup.config. I put more because at the beginning you provided all and I've juste adapted the configuration but as you explain people won't use those bundle so only one is required.

For the comment, you have to add the option removeComments: true inside the tsconfig.es5.json file and it should work as well like explain here.

And as I can see, you're not using all the plugins I told you last time - your config

You have to use all the plugins as I explained in my config I gave to you last time because you forgot commonjs, nodeResolve and json plugins and it's not good like this so check my previous comment and adapt the config please and like this all will work fine.

akanass commented 3 years ago

I did the PR to update the config

MasterKale commented 3 years ago

Thank you for the PR, the way you called the commonjs() plugin in the rollup config did the trick!

As for the TS copyright notice, I think it's coming in from TypeScript's tslib dependency! The ES5 bundle's sourcemap points to "./../node_modules/tslib/tslib.es6.js", and when I open up that file what do I see but a massive copyright notice:

Screen Shot 2021-04-05 at 8 48 17 AM

From some quick googling it seems TypeScript pulls in tslib when the build target is low enough, it's a "runtime" for its downlevel fixes. I might need to create a simple "cleanup" script to do a single find-and-replace to nuke that notice from the bundle.

MasterKale commented 3 years ago

I've got a PR out with updates addressing this issue: https://github.com/MasterKale/SimpleWebAuthn/pull/117

Can someone clarify for me, was the point of all this to support IE10 or IE11? If it's IE11 I think there's an opportunity to raise the "target" of the ES5 build to ES6. Otherwise I'll leave things as-is (despite the ES5 UMD bundle now jumping up to 11KB 😱)

akanass commented 3 years ago

As I said in the PR let me do some update on it tonight because I would like to check something and I may change the way to do it because we want only for browser so all can be handle by rollup. If you can wait a little bit it would be nice

MasterKale commented 3 years ago

I intentionally marked the PR as a Draft because I figured there'd be some feedback, and I've got one or two more things I want to try out before I commit to this new setup 🙂

Moumouls commented 3 years ago

Hi @akanass @MasterKale it seems that we have some activity here 🙂 After some reading here, and @MasterKale last comment on https://github.com/MasterKale/SimpleWebAuthn/pull/117 may be i need to wait your last changes before testing on my side env.

Also if you want to try the lib on old navigators, https://www.browserstack.com/ have free plan for open source projects, and they have also a nice feature that allow to share your localhost with the cloud navigator. it could help for testing purpose 🙂

akanass commented 3 years ago

@Moumouls just wait please I am finishing my part and it will be merged after and you will be able to test it.

I just have to finish the documentation because all build optimisations are done

You can see all in #116 and follow my WIP

Moumouls commented 3 years ago

no problem @akanass i wait the green light 🙂

MasterKale commented 3 years ago

@Moumouls thank you for telling me about BrowserStack's free sub for open source! 🎉 image

This'll make it much easier to test things out. Hopefully their localhost support has become a little more turnkey since the last time I used it 😀

Moumouls commented 3 years ago

Also @MasterKale if you have some hard time using their locahost system, you can use try to use https://www.npmjs.com/package/localtunnel, then you can expose your local port under a temporary https domain, https particularly useful since webauthn is only available under localhost or https 🙂

MasterKale commented 3 years ago

I took the new bundles for a spin via BrowserStack:

IE 11

swan-browser-ie11-es5-umd

Edge 17

swan-browser-edge17-es5-umd

They're just about ready 😎

MasterKale commented 3 years ago

I found some time this morning to test out the browser package in Create-React-App and managed to pretty easily get something working in IE10. I can confirm the new bundle strategy works just fine - I got a site running and using supportsWebAuthn() in IE10 (way lower than I think it needs to go, I just wanted to see how far back I could take things):

Screen Shot 2021-04-08 at 8 31 52 AM

And so that's it, we're all good! I've merged PR #117 and will cut a new release tonight.

Shout out to @akanass for helping me make sense of Rollup, and for contributing to the effort to keep @simplewebauthn as simple to use as possible 🙇

MasterKale commented 3 years ago

Alright, I just published a new release - ES5 support in @simplewebauthn/browser is now available as of v2.2.1. Check out the updated README for new UMD and build instructions to ensure your project pulls in the correct version:

https://github.com/MasterKale/SimpleWebAuthn/blob/master/packages/browser/README.md

Moumouls commented 3 years ago

Okay, i'll give it a try today on our projects, thanks @akanass and @MasterKale for your help on this one ! 🙂

Moumouls commented 3 years ago

Okay so out of the box with Next JS (with TS ) it seems that it do not work.

error - /Users/xxxxx/Desktop/Repos/xxxx/main/node_modules/@simplewebauthn/browser/dist/es2018/index.js:111
export { startAssertion, startAttestation, supportsWebauthn };
^^^^^^

SyntaxError: Unexpected token 'export'
    at wrapSafe (node:internal/modules/cjs/loader:999:16)
    at Module._compile (node:internal/modules/cjs/loader:1047:27)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1112:10)
    at Module.load (node:internal/modules/cjs/loader:948:32)
    at Function.Module._load (node:internal/modules/cjs/loader:789:14)
    at Module.require (node:internal/modules/cjs/loader:972:19)
    at require (node:internal/modules/cjs/helpers:88:18)
    at eval (webpack-internal:///@simplewebauthn/browser:1:18)

I'll try to make it work with the example provided on the Readme 🙂

On next i tried:

    webpack(config, { isServer }) {
        console.log(config.resolve.mainFields)
        // config.resolve.mainFields = ['browser', 'module', 'main']
        config.resolve.alias['@graphql'] = `${__dirname}/graphql/generated.tsx`

even with config.resolve.mainFields = ['browser', 'module', 'main'] un commented, the issue persist

88% hashingready - started server on 0.0.0.0:3000, url: http://localhost:3000
93% after chunk asset optimization SourceMapDevToolPluginwarn  - React 17.0.1 or newer will be required to leverage all of the upcoming features in Next.js 11. Read more: https://err.sh/next.js/react-version
95% emitting WatchMissingNodeModulesPlugin[ 'browser', 'module', 'main' ]
[ 'main', 'module' ]

You can see in the log that Next seems to have 2 config one browser and the other for SSR. first log [ 'browser', 'module', 'main' ] second log [ 'main', 'module' ]