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

Add main interface for library #450

Merged
merged 2 commits into from
Feb 8, 2020
Merged

Conversation

metcoder95
Copy link
Contributor

As requested in the following Issue: #448

The PR exports a main interface for the dotenv library.

@jcblw
Copy link
Collaborator

jcblw commented Feb 1, 2020

@metcoder95 looks great and thanks for the PR 🎉

types/index.d.ts Outdated
@@ -55,5 +55,12 @@ export interface DotenvConfigOutput {
*
*/
export function config(options?: DotenvConfigOptions): DotenvConfigOutput;

/** dotenv library interface */
export interface dotenv {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we export out this interface with the casing DotEnv? It would then match the casing of the other interface here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

actually I think i am wrong about this let me download the code and get back to you.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok pulled down, a saw the behavior of this. I think we should Pascal case the interfaces. I would love for the typing to work with.

import dotenv from 'dotenv'

dotenv.foo() // throws on compile.
dotenv.config() // works 

I wish I was better at typescript and could add some code of how to do this, but sadly I am not quite sure how to get this to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To do that, we can use namespace to make it 👍

Copy link
Collaborator

@jcblw jcblw left a comment

Choose a reason for hiding this comment

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

Can you just update the interface to be Pascal case and I think this is good to go.

@metcoder95
Copy link
Contributor Author

Done! 👍
Sorry for the delay, I was quite busy.
Let me know if you need more changes or comments

@metcoder95 metcoder95 requested a review from jcblw February 2, 2020 22:01
@jcblw jcblw merged commit 7301ac9 into motdotla:master Feb 8, 2020
@motdotla motdotla mentioned this pull request May 5, 2021
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