dwyl / sendemail

πŸ’Œ Simplifies reliably sending emails from your node.js apps using AWS Simple Email Service (SES)
181 stars 26 forks source link

Breaking change on 2.7.0 #59

Closed otajor closed 7 years ago

otajor commented 7 years ago

Problem

Changes to the way the module exports its functions seem to cause breaking changes due to #56. Instead of exporting the sendemail function like this:

module.exports = sendemail;

It is now being exported like this:

module.exports.email = sendemail;

This causes the error TypeError: email is not a function if your code is still expecting the old version.

How to fix

The issue can be fixed by changing how you import sendemail to reflect the above: Replace

const email = require('sendemail')
email.set_template_directory('./path/to/templates')

with

const { email, set_template_directory } = require('sendemail')
set_template_directory('./lib/emails/templates')

Or if you're on an older version of node:

const sendemail = require('sendemail')
const email = sendemail.email
const set_template_directory = sendemail.set_template_directory
set_template_directory('./lib/emails/templates')

Alternatively, change your package.json to specify a version older than 2.7.0 - maybe a better fix if you're using this in lots of different files.

nelsonic commented 7 years ago

@shouston3 can you help with this? (I think @otajor is right!) πŸ€”

otajor commented 7 years ago

I think this is the desired/expected behaviour in order to be able to stub easily with sinon.stub(sendemail, 'email') - I don't think it's an unintentional bug.

But should probably have been pushed as v3.0.0 rather than v2.7.0. Maybe @shouston3 could have been clearer that this was a breaking change?

nelsonic commented 7 years ago

@otajor agreed. let's do that. I'm AFK right now but if anyone has time to write a quick PR we can release a v3 tonight so people aren't "hurt" by this... thanks!

samhstn commented 7 years ago

Sorry, I upgraded the readme, but forgot to upgrade the package version

nelsonic commented 7 years ago

@shouston3 definitely not your fault. all you did was make this package bettererererer!! 😍 It's 100% my bad for not paying closer attention when reviewing the PR. πŸ˜– We're just lucky @otajor is "on the ball"... β˜”οΈ πŸ₯‡

Danwhy commented 7 years ago

Is it possible to unpublish versions on npm? Because 2.7.0 still exists with the breaking change, it's still automatically upgrading versions 2.6.0 and below and breaking them

nelsonic commented 7 years ago

@Danwhy oh... dang! I don't think there is a way to unpublish anymore since the LeftPad Fiasco... What would be a good course of action here...? πŸ€”

Danwhy commented 7 years ago

@nelsonic It seems if the release is more than 24 hours old, you have to contact npm support

nelsonic commented 7 years ago

@Danwhy I have sent them an email: image This should be done shortly. Thanks again for letting us know this was hurting you. πŸ‘ 😞

roryc89 commented 7 years ago

I don't know if this is now possible but it may be best to release another minor change that reverts the API back to before the major change was made. That way any users of the module with a ^ that automatically upgrades to the most recent minor change will not upgrade to the breaking change.

nelsonic commented 7 years ago

@roryc89 what is your NPM username? (happy for someone else to resolve this...)

roryc89 commented 7 years ago

Hi @nelsonic it's roryc89

nelsonic commented 7 years ago

@roryc89 you can publish a new version on the 2.x.x stream https://www.npmjs.com/package/sendemail please let me know if you need help with this... πŸ‘