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.

Sign up for GitHub

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

Jump to bottom

MessageAttachment Typescript Type #254

Closed
wyattjoh opened this issue Jun 9, 2020 · 5 comments
Closed

MessageAttachment Typescript Type #254

wyattjoh opened this issue Jun 9, 2020 · 5 comments

Comments

@wyattjoh
Copy link
Contributor

wyattjoh commented Jun 9, 2020

Your examples show the following:

// ...
const message = {
        // ...
	attachment: [
		{ data: '<html>i <i>hope</i> this works!</html>', alternative: true }
	],
};

client.send(message);
// ...

Yet the typescript type for the MessageAttachment does not allow that, causing you to do the following in your tests:

emailjs/test/message.ts

Lines 166 to 169 in 0781d02

attachment: ({
data: text,
alternative: true,
} as unknown) as MessageAttachment,

It's amazing that this project exports typescript types, but it would be helpful to correct this type so that you don't have this complexity of having to type cast the attachment.
@zackschuster
Copy link
Collaborator

@wyattjoh thanks for your feedback! i'm glad the types are useful for you :)

i went over the MessageAttachment interface again and managed to fix this. that in turn revealed the stream property was improperly typed — it should've been ReadStream, not Duplex — so double-kudos for pointing this out!

as a bonus, i was also able to merge it with AlternateMessageAttachment, so that's one less thing to keep track of :) i think it's ok to ship that as semver-patch since, as you pointed out, the types were essentially unusable before.

you can review these changes from the following commit:

  • smtp/message: fix attachment interface usability 19bda25

@eleith would you like to ship this now or after #252 is done? are you ok with it being semver-patch?

@eleith
Copy link
Owner

eleith commented Jun 9, 2020

let's do it after #252 if you think we can get this in this week?

@zackschuster
Copy link
Collaborator

yes, that's doable

@wyattjoh
Copy link
Contributor Author

wyattjoh commented Jun 9, 2020

Thank you for the fast response!! 👏

@zackschuster
Copy link
Collaborator

@wyattjoh as of 3.2.0 you should be able to use the code without type errors. you can also drop the typeRoots workaround in tsconfig.json, if you had included that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@eleith @wyattjoh @zackschuster