-
Notifications
You must be signed in to change notification settings - Fork 0
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 types #8
Add types #8
Conversation
|
||
function Authentic(opts: AuthenticOpts): Validator | ||
|
||
namespace Authentic { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason for the namespace here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to use a namespace so we're able to use a namespace import
import * as Authentic from "@articulate/authentic";
This allows users to still be able to do
import * as Authentic from "@articulate/authentic"
Authentic() // function
Authentic.JWT // interface
import authentic from "@articulate/authentic"
authentic() // function
import { JWT } from "@articulate/authentic"
JWT // interface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without the namespace, you'll get an error in typscript: Cannot invoke an expression whose type lacks a call signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left one question, but I think this looks good otherwise.
Not sure if you saw this, but I found this template on the typescript site: https://www.typescriptlang.org/docs/handbook/declaration-files/templates/module-function-d-ts.html which I think would be want we want here except for the fact that we're exporting the function using module.exports
instead of export =
. This seems like a good first pass, though, since I don't think we're interested in changing the actual code if we can avoid it right now.
@vscerchia Yeah I'm trying to leave the code alone with this PR. I'd love to make that suggestion in another version 👍 |
This PR adds a
index.d.ts
file to be used for typescript.