Skip to content

Replace moment.js with custom date generator #223

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

Merged
eleith merged 10 commits into from
Jun 24, 2018
Merged

Replace moment.js with custom date generator #223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
30d127c
add lockfile
zackschuster Jun 22, 2018
06e844b
use custom rfc2822 date generator
zackschuster Jun 22, 2018
b92ae52
drop moment
zackschuster Jun 22, 2018
4f5db54
update deps
zackschuster Jun 22, 2018
5a074d1
add utc date generator
zackschuster Jun 22, 2018
4ffeaf2
fix tests for varying timezones
zackschuster Jun 22, 2018
8053ae0
fix missing test for getRFC2822Date
zackschuster Jun 23, 2018
0a9bfd6
use regex for date tests
zackschuster Jun 24, 2018
557c480
move date functions to separate files
zackschuster Jun 24, 2018
8ccd207
use super-fancy destructuring, because i can
zackschuster Jun 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,4 @@
*.swo
*~

yarn.lock
package-lock.json
1 change: 1 addition & 0 deletions email.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
exports.server = require('./smtp/client');
exports.message = require('./smtp/message');
exports.date = require('./smtp/date');
exports.SMTP = require('./smtp/smtp');
exports.error = require('./smtp/error');
15 changes: 7 additions & 8 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,19 @@
},
"dependencies": {
"addressparser": "^0.3.2",
"emailjs-mime-codec": "^2.0.5",
"moment": "2.20.1",
"starttls": "1.0.1"
"emailjs-mime-codec": "^2.0.7",
Copy link
Owner

@eleith eleith Jun 23, 2018

Choose a reason for hiding this comment

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

before there was a yarn/package.json lock file, i had some experience with even minor bumps introducing bugs that impacted deployments that did not intend to have anything upgraded.

now with locks, i don't see that happen as much. what are you thoughts on it? for dev dependencies, i don't think it matters as much, but for direct dependencies, it's worth thinking about.

since you removed moment and changed startls to be 'compatible' they are now all consistent. so that is good.

let me know what you think and we can run with this or to be safe we can change them all to be locked to one version.

Copy link
Collaborator Author

@zackschuster zackschuster Jun 24, 2018

Choose a reason for hiding this comment

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

personally my stance is, now that lockfiles are normalized, an end user can and should be able to ensure their builds are reproducible without libraries needing to pin themselves -- in fact, libraries should avail themselves of all the latest versions of libraries they can take without breaking tests. that's the best way (in my mind) to ensure the most secure and bug-free code is shipped.

that being said, latest version of node 6 ships with npm 3.10.3, and package-lock wasn't introduced until npm 5 -- i wouldn't want to dismiss people in that situation out-of-hand, knowing that e.g. joyent will still be on 0.10 for some time.

i wonder, though, if people in that situation would even bother touching their dependencies, especially if the next release is cut as semver major. it might be a moot point altogether, in which case i fall back to my original argument.

"starttls": "^1.0.1"
},
"devDependencies": {
"chai": "1.1.0",
"chai": "^4.1.2",
"eslint": "^4.19.1",
"eslint-config-prettier": "^2.9.0",
"eslint-plugin-mocha": "^5.0.0",
"eslint-plugin-prettier": "^2.6.0",
"mailparser": "2.2.0",
"mocha": "5.0.0",
"prettier": "^1.12.1",
"smtp-server": "^3.4.1"
"mailparser": "^2.2.0",
"mocha": "^5.2.0",
"prettier": "^1.13.5",
"smtp-server": "^3.4.6"
},
"engine": [
"node >= 6"
Expand Down
29 changes: 29 additions & 0 deletions smtp/date.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
function getRFC2822Date(date = new Date(), useUtc = false) {
if (useUtc) {
return getRFC2822DateUTC(date);
}

const dates = date
.toString()
.replace('GMT', '')
.replace(/\s\(.*\)$/, '')
.split(' ');

dates[0] = dates[0] + ',';

const day = dates[1];
dates[1] = dates[2];
dates[2] = day;

return dates.join(' ');
}

function getRFC2822DateUTC(date = new Date()) {
const dates = date.toUTCString().split(' ');
dates.pop(); // remove timezone
dates.push('+0000');
return dates.join(' ');
}

exports.getRFC2822Date = getRFC2822Date;
exports.getRFC2822DateUTC = getRFC2822DateUTC;
7 changes: 3 additions & 4 deletions smtp/message.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ const { Stream } = require('stream');
const fs = require('fs');
const os = require('os');
const path = require('path');
const moment = require('moment');
const mimeWordEncode = require('emailjs-mime-codec').mimeWordEncode;
const addressparser = require('addressparser');
const { getRFC2822Date } = require('./date');

const CRLF = '\r\n';
const MIMECHUNK = 76; // MIME standard wants 76 char chunks when sending out.
const MIME64CHUNK = MIMECHUNK * 6; // meets both base64 and mime divisibility
Expand Down Expand Up @@ -48,9 +49,7 @@ class Message {
'message-id': `<${new Date().getTime()}.${counter++}.${
process.pid
}@${os.hostname()}>`,
date: moment()
.locale('en')
.format('ddd, DD MMM YYYY HH:mm:ss ZZ'),
date: getRFC2822Date(),
};

this.content = 'text/plain; charset=utf-8';
Expand Down
41 changes: 41 additions & 0 deletions test/date.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
describe('rfc2822 dates', function() {
const { expect } = require('chai');
const {
date: { getRFC2822Date, getRFC2822DateUTC },
} = require('../email');

var d_utc = dt => getRFC2822DateUTC(new Date(dt));
var d = (dt, utc = false) => getRFC2822Date(new Date(dt), utc);

it('should match standard regex', function(done) {
// RFC 2822 regex: For details see https://tools.ietf.org/html/rfc2822#section-3.3
// thanks to moment.js for the listing: https://github.com/moment/moment/blob/a831fc7e2694281ce31e4f090bbcf90a690f0277/src/lib/create/from-string.js#L101
var rfc2822re = /^(?:(Mon|Tue|Wed|Thu|Fri|Sat|Sun),?\s)?(\d{1,2})\s(Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec)\s(\d{2,4})\s(\d\d):(\d\d)(?::(\d\d))?\s(?:(UT|GMT|[ECMP][SD]T)|([Zz])|([+-]\d{4}))$/;
expect(d(0)).to.match(rfc2822re);
expect(d(329629726785)).to.match(rfc2822re);
expect(d(729629726785)).to.match(rfc2822re);
expect(d(1129629726785)).to.match(rfc2822re);
expect(d(1529629726785)).to.match(rfc2822re);

done();
});

it('should produce proper UTC dates', function(done) {
expect(d_utc(0)).to.equal('Thu, 01 Jan 1970 00:00:00 +0000');
expect(d_utc(0)).to.equal(d(0, true));

expect(d_utc(329629726785)).to.equal('Thu, 12 Jun 1980 03:48:46 +0000');
expect(d_utc(329629726785)).to.equal(d(329629726785, true));

expect(d_utc(729629726785)).to.equal('Sat, 13 Feb 1993 18:55:26 +0000');
expect(d_utc(729629726785)).to.equal(d(729629726785, true));

expect(d_utc(1129629726785)).to.equal('Tue, 18 Oct 2005 10:02:06 +0000');
expect(d_utc(1129629726785)).to.equal(d(1129629726785, true));

expect(d_utc(1529629726785)).to.equal('Fri, 22 Jun 2018 01:08:46 +0000');
expect(d_utc(1529629726785)).to.equal(d(1529629726785, true));

done();
});
});
Loading