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

Create TypeScript typings #39

Closed
mstuart opened this issue Oct 2, 2020 · 21 comments
Closed

Create TypeScript typings #39

mstuart opened this issue Oct 2, 2020 · 21 comments
Assignees

Comments

@mstuart
Copy link
Contributor

mstuart commented Oct 2, 2020

This project isn't written in TypeScript, but many TypeScript users would like to use published typings. For this issue, create TypeScript typings that cover the exported functions, and contribute it back to DefinitelyTyped!

This issue is great for someone who wants to learn more about TypeScript!

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label feature_request to this issue, with a confidence of 0.96. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@ReazerDev
Copy link

Hi, I'm interested in this. Never made typings for ts, sounds interesting!

@claeusdev
Copy link

Ah @ReazerDev beat me to it. If you need an extra hand lemme know.

@mstuart
Copy link
Contributor Author

mstuart commented Oct 2, 2020

No worries @claeusdev! If you're wanting to hack around w/ TypeScript, there are some other similar ones under these repos too! https://github.com/paypal/hacktoberfest/blob/main/README.md#repos

@ReazerDev
Copy link

Wait, am I too eligible to get the socks?^^ I'm asking since I don't commit anything to this repo, but to DefinitelyTyped

@mstuart
Copy link
Contributor Author

mstuart commented Oct 3, 2020

@ReazerDev No problem! When you commit it to DefinitelyTyped, can you drop the link here?

@ReazerDev
Copy link

@ReazerDev
Copy link

Has been merged :)

@mstuart
Copy link
Contributor Author

mstuart commented Oct 3, 2020

@ReazerDev wow, that was really fast 😸 Thanks! We have you down for some fancy socks. Have fun hacking :)

@ReazerDev
Copy link

Nice :) Was a lot of fun, thank you. You too have fun^^

@ReazerDev
Copy link

Am I getting an email regarding shipping information?

@mstuart
Copy link
Contributor Author

mstuart commented Oct 3, 2020

I was thinking the same thing. I'm not sure how to get your shipping info. We probably should've included a google form or thought that through.

Can you add your email to your GitHub profile real quick? Then you can take it down afterwards. Open to other ideas too. Or maybe I can DM you on Twitter or another way? Hmm... 🤔

@ReazerDev
Copy link

My Twitter is: Reazer_Dev

@gregjopa
Copy link
Contributor

gregjopa commented Oct 5, 2020

@ReazerDev thanks so much for adding types for paypal-js to DefinitelyTyped! 👏

I was reviewing the PR and noticed a minor issue. The public api for paypal-js v1 is a single function loadScript() and a string attribute named version. The functions in utils.js are not publicly exported and should be removed from the typescript definitions. I could see how this is confusing since the paypal-js project lacks an index.js that clearly defines what is publicly exported.

Would you be up for sending a follow up PR to DefinitelyTyped to remove the types for the private functions? The types files should look like this:

// Type definitions for @paypal/paypal-js 1.0
// Project: https://github.com/paypal/paypal-js
// Definitions by: ReazerDev <https://github.com/ReazerDev>
// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped

export interface ScriptOptions {
  // options here
}

export function loadScript(options: ScriptOptions): Promise<any>;
export const version: string;

@ReazerDev
Copy link

@ReazerDev thanks so much for adding types for paypal-js to DefinitelyTyped! 👏

I was reviewing the PR and noticed a minor issue. The public api for paypal-js v1 is a single function loadScript() and a string attribute named version. The functions in utils.js are not publicly exported and should be removed from the typescript definitions. I could see how this is confusing since the paypal-js project lacks an index.js that clearly defines what is publicly exported.

Would you be up for sending a follow up PR to DefinitelyTyped to remove the types for the private functions? The types files should look like this:

// Type definitions for @paypal/paypal-js 1.0
// Project: https://github.com/paypal/paypal-js
// Definitions by: ReazerDev <https://github.com/ReazerDev>
// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped

export interface ScriptOptions {
  // options here
}

export function loadScript(options: ScriptOptions): Promise<any>;
export const version: string;

Oh yes, of course! Will do once I'm home!

@gregjopa
Copy link
Contributor

gregjopa commented Oct 5, 2020

Awesome! Thanks @ReazerDev!

Also, thanks for documenting all the scriptOptions. I think you got almost all of them. Here's a list I have on my end of all the different available options. It's in React PropTypes, but it should help with identifying any missing ones.

        "buyer-country": PropTypes.string,
        "client-id": PropTypes.string.isRequired,
        commit: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]),
        components: PropTypes.string,
        currency: PropTypes.string,
        "data-csp-nonce": PropTypes.string,
        "data-order-id": PropTypes.string,
        "data-page-type": PropTypes.string,
        "data-partner-attribution-id": PropTypes.string,
        debug: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]),
        "disable-funding": PropTypes.string,
        "integration-date": PropTypes.string,
        intent: PropTypes.string,
        locale: PropTypes.string,
        "merchant-id": PropTypes.string,
        vault: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]),

@ReazerDev
Copy link

@gregjopa
Copy link
Contributor

gregjopa commented Oct 5, 2020

Great improvements @ReazerDev! Thanks for taking care of this so quickly. I approved the PR and it looks like you can merge it whenever you're ready: DefinitelyTyped/DefinitelyTyped#48513 (comment)

@ReazerDev
Copy link

Yup merged it

@gregjopa
Copy link
Contributor

gregjopa commented Oct 5, 2020

Thanks again @ReazerDev. I verified the types locally by installing @types/paypal__paypal-js and it works great 😄

@gregjopa gregjopa closed this as completed Oct 5, 2020
@ReazerDev
Copy link

Great! Was fun working on it!

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

No branches or pull requests

4 participants