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

feature/client-typescript #25

Closed
wants to merge 39 commits into from
Closed

feature/client-typescript #25

wants to merge 39 commits into from

Conversation

MasterKale
Copy link

This PR includes a full TypeScript rewrite of the @webauthn/client library. The compiled version of the library still supports use in JavaScript projects, but I've also added the emitting of several .d.ts files that support Intellisense and autocompletion of all of the library's methods in both JavaScript- and TypeScript-based projects.

Webpack is still used to bundle everything into a single main.js as before. However, Babel has been completely replaced with ts-loader for simplified TypeScript configuration while editing and when compiling this library.

Lastly I've also updated the README (in the client/ folder specifically) to talk about the contents of this library specifically.

@MasterKale
Copy link
Author

This PR was partially inspired by #16.

I really like the direction this library is going and figured I'd try and contribute some improvements. I wasn't able to really gauge the level of interest in adding more TypeScript support (aside from that PR) so I apologize in advance if I'm overstepping by proposing such a drastic rewrite of half of this library 😅

@P4sca1
Copy link

P4sca1 commented Jan 22, 2020

Great work! Would be great to also do this for @webauthn/server.

Copy link
Contributor

@eheikes eheikes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @MasterKale. I noticed a couple minor things but otherwise looks good.

packages/client/package-lock.json Outdated Show resolved Hide resolved
packages/client/README.md Outdated Show resolved Hide resolved
@MasterKale
Copy link
Author

@P4sca1 That's my hope! I decided to tackle the client codebase first as it was a much lower LOE to convert to TS in a backwards-compatible way. If this PR gets in then some of the interfaces included will simplify porting the server codebase over as well ✌️

@MasterKale
Copy link
Author

I'm trying to get up to speed with Yarn workspaces and Lerna to make sure my work doesn't break the publishing workflow (it's my first time with either). However, I can't make heads or tails of the fact that, while workspaces seem to keep a single repo-wide yarn.lock file, this project also has yarn.lock files in both the client/ and server/ packages.

It seems what I need to do is delete client/yarn.lock, then include in this PR changes to the root yarn.lock that occur when I run yarn install. If not, what's the proper way to update the lock file to track my changes to client's dependencies?

@P4sca1
Copy link

P4sca1 commented Jan 25, 2020

Afaik there shouldn’t be a yarn.lock in client and server packages. There should only be one in the root directory.

@eheikes
Copy link
Contributor

eheikes commented Jan 26, 2020

Afaik there shouldn’t be a yarn.lock in client and server packages. There should only be one in the root directory.

I think that's legacy cruft that was never cleaned up in #17.

It seems what I need to do is delete client/yarn.lock, then include in this PR changes to the root yarn.lock that occur when I run yarn install.

You could also run lerna bootstrap, which delegates to yarn.

@MasterKale
Copy link
Author

MasterKale commented Jan 28, 2020

Afaik there shouldn’t be a yarn.lock in client and server packages. There should only be one in the root directory.

Great, thanks for clearing that up! I'll go ahead and clean up those two yarn.lock files, and then update the root one.

I've been pretty busy lately but I haven't forgotten about this! I do plan on getting these fixes in the next day or two :)

@MasterKale
Copy link
Author

Alright, sorry about the delay! I believe I've addressed all of the outstanding issues we talked about above. This includes removing both client/yarn.lock and server/yarn.lock directories, and an updated yarn.lock in the root of the project.

Barring any other suggested changes, this is ready to go!

@MasterKale
Copy link
Author

I'm bumping this so it doesn't get too stale...

@MasterKale MasterKale closed this May 24, 2020
@MasterKale MasterKale deleted the feature/client-typescript branch May 24, 2020 02:31
@P4sca1
Copy link

P4sca1 commented May 24, 2020

@MasterKale Have you found another package for Webauthn or why have you closed this pr?

@MasterKale
Copy link
Author

@P4sca1 I closed this PR because I'm releasing my own pair of front-end and back-end WebAuthn libraries under the "WebAuthntine" name:

https://github.com/MasterKale/WebAuthntine

I took some inspiration on the project's structure from this repo, but the code is entirely based on a side project I built last year while diving deep into the API to learn how it works. And best of all everything is 100% TypeScript!

@P4sca1
Copy link

P4sca1 commented May 24, 2020

This is great! I always thought about writing my own code for webauthn in typescript, but haven't found the time. Very excited about your project. I hope it gets more attention than this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants