fix: Correct TypeScript definitions by notheotherben · Pull Request #300 · sendgrid/sendgrid-nodejs · GitHub
Skip to content

fix: Correct TypeScript definitions#300

Merged
thinkingserious merged 1 commit into
sendgrid:masterfrom
notheotherben:fix/typescript-definitions
Sep 27, 2016
Merged

fix: Correct TypeScript definitions#300
thinkingserious merged 1 commit into
sendgrid:masterfrom
notheotherben:fix/typescript-definitions

Conversation

@notheotherben

@notheotherben notheotherben commented Sep 20, 2016

Copy link
Copy Markdown
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");
``

@spartan563
fix: Correct TypeScript definitions
872b5f7 
See sendgrid#251 and <notheotherben/sendgrid-ts-demo#1> for more details on what wasn't working
thinkingserious
thinkingserious reviewed Sep 22, 2016
View reviewed changes

@thinkingserious thinkingserious left a comment

Copy link
Copy Markdown
Contributor

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!

Comment thread index.d.ts

@thinkingserious thinkingserious Sep 22, 2016

Copy link
Copy Markdown
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.

@notheotherben notheotherben Sep 22, 2016

Copy link
Copy Markdown
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

thinkingserious commented Sep 22, 2016

Copy link
Copy Markdown
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

Javierenrique00 commented Sep 23, 2016

Copy link
Copy Markdown

Could you please add an example using TypeScript in NodeJS ?

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

ss108 commented Oct 4, 2016

Copy link
Copy Markdown

Hey,

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

@notheotherben

notheotherben commented Oct 4, 2016

Copy link
Copy Markdown
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
@notheotherben
fix: Allow emptyRequest() to be called with a request object
166580c 
Fixes an issue raised in sendgrid#300
@ss108

ss108 commented Oct 4, 2016

Copy link
Copy Markdown

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

Reviewers

@thinkingserious thinkingserious thinkingserious left review comments

Assignees

No one assigned

Labels

type: bug bug in the library

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@notheotherben @thinkingserious @Javierenrique00 @ss108 @spartan563