eladnava/mailgen

#18 contains breaking changes to custom themes

petetnt opened this issue ยท 5 comments

PR #18 introduced the ability to add multiple buttons. Despite there being no API changes, sadly this breaks every custom theme out there that uses action.button directly in the templates, like it was used in default and cerberus themes before the update.

We learned this the hard way when a SemVer patch update 1.0.27 broke all of our email related functionality. Not sure what versioning scheme mailgen follows, but as npm relies on packages following SemVer, this should have prompted a SemVer major style bump to 2.0.0.

Solutions for this issue would be either

  • Bumping the SemVer so the breaking change wouldn't affect those relying on already working versions

or

  • Changing the way #18 is handled so that actions can be arrays or objects internally too.

Hi @petetnt,
Thanks so much for bringing this to our attention! I have immediately published a new patch version without the PR merged in temporarily.

Later I will re-publish a new version with the PR included, probably as a major version. I do not have the time right now.

Thanks @eladnava ๐Ÿ‘

The only alternative to publishing #18 as a major version would be to have Mailgen provide the themes with locals.action as an object if there is only one action to be printed, and an array otherwise. The existing custom themes that expect an object and don't support more than one action will work fine this way.

However, this would unfortunately cause us to pollute the default themes that support more than one action with code that will convert the action object into an array, in every single theme .html and .txt file, which will be repetitive and ugly.

So I think the best course of action is to publish #18 as a major version instead, as you suggested.

Thanks again @petetnt for bringing this to our attention, I do apologize that this was not thought of beforehand and hope that the damage done was not too severe.

Published v2.0.0.

No problem @eladnava, thanks for the prompt actions ๐Ÿ‘