search

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

Fixed invalid format of ES modules that caused compiling issues for Vite #128

Closed skoshx closed 3 years ago

skoshx commented 3 years ago

This pull request fixes an issue where @simplewebauthn/browser cannot be imported, because it causes Vite to error because Vite doesnt look for the main:es2018, but instead looks for module, which it can't find.

Moumouls commented 3 years ago

Hi there, i'll be happy to test it in our env, tell when you think this branch is ready 🙂

Note: @MasterKale if we do not have any issue on my NEXTJS repro repo, we should be okay !

MasterKale commented 3 years ago

Note: @MasterKale if we do not have any issue on my NEXTJS repro repo, we should be okay !

@Moumouls Thank you for reminding me about your repro repo (https://github.com/Moumouls/next-simplewebauthn), I'm happy to report that with Skoshx' PR change your NextJS repro app loaded the ES2018 module before outputting a build capable of executing in IE10:

Screen Shot 2021-06-17 at 7 18 12 AM

This seals the deal. Once this PR gets merged I'm replacing the ES5 bundle in "main" with the ES2018 bundle so that the browser package goes back to being a simple npm install to use.

akanass commented 3 years ago

@MasterKale we have nothing to do as I understand it but just put the ES2018 version in the main and that's it, right?

This PR isn't required if we only have the main now?

akanass commented 3 years ago

@MasterKale I can do all the cleaning and only have the ES2018 version in main entry because we don't need module entry as it works directly with the ES2018

And if everything works with the version ES2018 it means that the build we created was good and that we should have tested only the new version ES2018 to not have the one in ES5

akanass commented 3 years ago

@MasterKale I've created the PR #129 to clean all and only have the ES2018 version in the main entry

If all is working with my PR, this one should be closed.

MasterKale commented 3 years ago

@skoshx Thank you for submitting this PR and helping to make SimpleWebAuthn that much more simple to use. As you're aware this PR was superseded by PR #129, so I'm going to close this one out.

One final comment: I apologize for not encouraging @akanass to contribute to this PR instead so you'd get Contributor credit as well. I hope this won't discourage you from contributing in the future 🙇

skoshx commented 3 years ago

@MasterKale Yeah, no problem! And as nice at would be to be amongst the list of contributors, I'm just happy that I could at least shed some light into this situation, and help make it better. This repository has been an immense help with my own project, so it's the least I can do :)