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.

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

Remove moment #231

Merged
merged 4 commits into from
Mar 22, 2024
Merged

Remove moment #231

merged 4 commits into from
Mar 22, 2024

Conversation

rowanmanning
Copy link
Member

Slight side-track, but oh well, here we are! We don't need Moment for these use-cases, and they're probably clear enough without needing any date manipulation/parsing libraries.

Date addition / subtraction

We can add/subtract using regular date. The following code to subtract seconds is equivalent:

function runMoment(samplePeriod) {
  const timeWindow = samplePeriod * 1.5;
  const now = moment();
  const startTime = now.subtract(timeWindow, 'seconds');
  return startTime.toISOString();
}

function runDate(samplePeriod) {
  const timeWindowInMs = (samplePeriod * 1.5) * 1000;
  const now = new Date();
  const startTime = new Date(now.getTime() - timeWindowInMs);
  return startTime.toISOString();
}

console.log("moment", runMoment(60 * 5));
console.log("js", runDate(60 * 5));

The following code to add two weeks to a date is equivalent:

function runMoment() {
  const now = moment();
  const limitDate = moment().add(2, 'weeks');
  return limitDate.toISOString();
}

function runDate() {
  const now = new Date();
  const limitDate = new Date();
  limitDate.setDate(now.getDate() + (2 * 7));
  return limitDate.toISOString();
}

console.log("moment", runMoment());
console.log("js", runDate());

Date comparison

Dates can be compared with regular operators:

console.log(new Date('2023-01-01') > new Date('2024-01-01'));

Date parsing

Date parsing is generally fine too especially in ISO format, we can do a little bit of extra validation but tbh I don't think it's needed - we can just trust the date format that the Fastly API returns (it's the only place this date can come from). These are the same:

console.log(moment("2024-01-26T10:00:00Z", 'YYYY-MM-DDTHH:mm:ssZ').toISOString());
console.log(new Date("2024-01-26T10:00:00Z").toISOString());

Moment really isn't needed for this. We can subtract using regular date
objects and timestamps. The following code is a proof:

```js
function runMoment(samplePeriod) {
  const timeWindow = samplePeriod * 1.5;
  const now = moment();
  const startTime = now.subtract(timeWindow, 'seconds');
  return startTime.toISOString();
}

function runDate(samplePeriod) {
  const timeWindowInMs = (samplePeriod * 1.5) * 1000;
  const now = new Date();
  const startTime = new Date(now.getTime() - timeWindowInMs);
  return startTime.toISOString();
}

console.log("moment", runMoment(60 * 5));
console.log("js", runDate(60 * 5));
```

The output values are almost exactly the same, the 1-2ms difference can
be accounted for by the Moment code taking a while to run.
This really isn't needed, Moment.js is a massive library and you can do
the equivalent with plain JavaScript nowadays.

Firstly, date comparison. Dates can be compared with regular operators:

```js
console.log(new Date('2023-01-01') > new Date('2024-01-01'));
```

Date parsing is generally fine too especially in ISO format, we can do a
little bit of extra validation but tbh I don't think it's needed - we
can just trust the date format that the Fastly API returns. Example of
date parsing just working:

```js
console.log(moment("2024-01-26T10:00:00Z", 'YYYY-MM-DDTHH:mm:ssZ').toISOString());
console.log(new Date("2024-01-26T10:00:00Z").toISOString());
```

Lastly there's no need to use Moment for date addition. The following
are equivalent:

```js
function runMoment() {
  const now = moment();
  const limitDate = moment().add(2, 'weeks');
  return limitDate.toISOString();
}

function runDate() {
  const now = new Date();
  const limitDate = new Date();
  limitDate.setDate(now.getDate() + (2 * 7));
  return limitDate.toISOString();
}

console.log("moment", runMoment());
console.log("js", runDate());
```
Nothing wrong with the date stuff, just some silly logic errors
@rowanmanning rowanmanning enabled auto-merge (rebase) March 21, 2024 11:05
@rowanmanning rowanmanning merged commit ed7d1f6 into main Mar 22, 2024
11 checks passed
@rowanmanning rowanmanning deleted the remove-moment branch March 22, 2024 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants