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

fix: Correct TypeScript definitions #300

Merged

Conversation

notheotherben
Copy link
Contributor

See #251 and notheotherben/sendgrid-ts-demo#1 for more details on what wasn't working.

This ensures that the following code works correctly:

import * as SendGrid from "sendgrid";

const sendGrid = SendGrid("MYAPIKEY");
``

See sendgrid#251 and <notheotherben/sendgrid-ts-demo#1> for more details on what wasn't working
Copy link
Contributor

@thinkingserious thinkingserious left a comment

Choose a reason for hiding this comment

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

Could you please explain how these changes fix the TypeScript definitions?

Thanks!

export = sendGrid;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking this would be a breaking change. SendGrid is the expected case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is certainly a breaking change when it comes to the definition, however it corrects the definition to match the actual behaviour of the SendGrid JS library.

I've updated the interface above to include the rest of the library's exports as expected.

@thinkingserious
Copy link
Contributor

@spartan563,

Thanks for the quick turnaround on the fix! I just had a few points of clarification in the review. We should be able to merge this one quickly.

@Javierenrique00
Copy link

Could you please add an example using TypeScript in NodeJS ?

@thinkingserious thinkingserious merged commit 34b0f1c into sendgrid:master Sep 27, 2016
@ss108
Copy link

ss108 commented Oct 4, 2016

Hey,

Sorry to comment on a closed PR, but doesn't emptyRequest need to take an object as a parameter?

@notheotherben
Copy link
Contributor Author

Yeah, you're correct - sorry I missed that, let me quickly throw together a new PR

notheotherben added a commit to notheotherben/sendgrid-nodejs that referenced this pull request Oct 4, 2016
@ss108
Copy link

ss108 commented Oct 4, 2016

I forked this and made a couple other changes to the definition: https://github.com/bloomtri/sendgrid-nodejs

The callback for the API method should be optional, and it needs to return a Promise. If those look good to you, then please incorporate them and I can delete that fork.

Thanks for getting to this so promptly!

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

Successfully merging this pull request may close these issues.

5 participants