moment.js -> date-fns migration about js HOT 9 CLOSED

Is there some way we can handle these cases right now?

Yeah we should only be using the rule when the original thing is a simple identifier.

The simple case, where we already have an identifier to use:

• date.add(10, "days") => date = addDays(date, 10)

Where it's a more complex expression, we can introduce a new variable and leave a TODO comment

From:

`user.getAge().add(1, "d");`

To:

// TODO: date-fns are immutable - feed this value back through properly
const newDate = addDays(user.getAge(), 1)

Should we move it to the stdlib documentation?

Yeah it used to be there, should definitely add it back.

from js.

Assigned to @srijan-paul.

from js.

So far, I've been able to migrate:

• constructing dates
• formatting dates
• performing arithmetic on dates
• UTC/Locale utils.
• Querying

However, I've run into a few problems that I don't think can be addressed with GritQL alone.

Mutation of date objects in moment.js

Moment.js uses its own representation of dates, whereas date-fns works with the JavaScript Date object.
Moment.js Date objects are mutable.
Given this, consider the following code snippet:

const date = moment("2001-01-01");
// calling `days` with a parameter mutates the moment date object.
date.days(10);
console.log(d.format("YYYY-MM-DD"))
// logs: 2001-01-10

The equivalent date-fns code would be:

const date = new Date("2001-01-01")
const nextDate = addDays(date, 10)
console.log(format(nextDate, "YYYY-MM-DD"))
// logs: 2001-01-10

One could wrangle with the possible block structure around the days expression and introduce a new variable,
but I don't think there exists a way to check if a free variable name has already been taken by the script in GritQL yet.

Adding durations to dates has the same problem:

// in momentjs, `add` mutates the date object itself:
date.add(10, "days") // -> mutates and returns `this`

// in datefns, `add` returns a new date:
add(date, { days: 10 }) // -> Date

Replacing the first expression with the second is trivial,
but the semantics are entirely different.
Any suggestions on how I could address this?

Bulk imports

Moment.js has a single default export.
date-fns is optimized for tree-shaking bundlers, and therefore
exports its helper functions in small modules.

moment.js:

import moment from "moment"

const date = moment("2001-01-01")
const nextDate = date.add(1, )

date-fns:

import fns from "date-fns"
import add from "date-fns/add"

const date = new Date("2001-01-01")
const nextDate = add(date, { days: 10 })

Currently, I'm replacing a single moment import with all necessary imports from date-fns,
but with this approach, the user's code would end up with unused imports.

Here is a small sample on how simple constructors/arithmetic can be migrated:

pattern moment_arith($date, $method, $count, $pat) {
    `$date.$method($count, $pat)` where {
        $method <: r"add|sub",
        $pat <: string()
    }
}

`$moment` where {
    $moment <: or {
        // replace imports
        `const $id = require("moment")` => `const dateFns = require("date-fns");\nconst add = require("date-fns/add");\nconst sub = require("date-fns/sub")`,

        // display
        `moment($date, $fmt)` => `dateFns.format($date, $fmt)`,
        `$date.format($fmt)` => `dateFns.format($date, $fmt)`,

        // construction
        `moment($arg)` => `new Date($arg)`,

        // add/subtract
        moment_arith($date, $method, $count, $pat) => `$method($date, { $pat: $count })` where {
            $pat <: or {
                string(fragment=r"\b(?:y|years?)\b") => `years`,
                string(fragment=r"\b(?:q|quarters?)\b") => `quarters`,
                string(fragment=r"\b(?:M|months?)\b") => `months`,
                string(fragment=r"\b(?:w|weeks?)\b") => `weeks`,
                string(fragment=r"\b(?:d|days?)\b") => `days`,
                string(fragment=r"\b(?:h|hours?)\b") => `hours`,
                string(fragment=r"\b(?:m|minutes?)\b") => `minutes`,
                string(fragment=r"\b(?:s|seconds?)\b") => `seconds`,
            }
        },
        moment_arith($date, $method, $count, $pat) where {
            $pat <: string(fragment=$frag where { $frag <: r"\b(?:ms|milliseconds?)\b"})
        } => `$method($date, { seconds: $count / 1000 })`
    }
}

from js.

Mutation of date objects in moment.js

I think the simplest change is to change the code to convert the mutating calls into assignment expressions. We should also attempt to (on a best effort basis) backtrack to the original definition and make it a let.

Your example would become:

let date = moment("2001-01-01");
// calling `days` with a parameter mutates the moment date object.
date = addDays(date, 10)
console.log(format(date, "YYYY-MM-DD"))

Bulk imports
We should not need to import unused functions. We have ensure_import_from built into [the standard library(https://github.com/getgrit/js/blob/main/.grit/patterns/importing.md) so we should be able to use that to import the proper functions when they are actually used.

So something like this:

// remove all moment imports
remove_import(from=`"moment"`)
// When we use a function from date-fns, import it
$fn = `add`,
$fn <: ensure_import_from(source=`"date-fns/add"`)

from js.

I think the simplest change is to change the code to convert the mutating calls into assignment expressions

Yes, I had tried this pattern for re-writing all let declarations (ideally we should only re-write declarations that are mutated, like you mentioned; I'll make that change):

pattern moment_var_decl($moment, $declaration, $declarators) {
    `$varName = $moment()` where {
        $moment <: `moment`,
        $varName <: within `$declaration` where { $declaration <: `const $declarators` },
    }
}

moment_var_decl($moment, $declaration, $declarators) where {
    $moment => `new Date()`,
    $declaration => `let $declarators`
}

However, not all mutating calls are going to be valid LHSs for an assignment expression.
Consider this:

// returns a mutable reference to date that cannot be re-assigned.
function make_date() {
  let date = moment(); // <- already a let binding
  return { get() { return date; } }
}

const date = make_date();
date.get().add(10, "days");

A rewrite (using raw) would yield invalid JavaScript:

date.get() = date.get().add(10, "days")

Something like this might be a part of any moment.js codebase:

class User {
  constructor(date) { this.age = moment(date); }
  getAge() { return this.age; }
}

// increase age:
user.getAge().add(1, "d");

Any sufficiently advanced application that uses date-manipulation could have a pattern like the one above.
Is there some way we can handle these cases right now?

We should not need to import unused functions. We have ensure_import_from built into [the standard library(https://github.com/getgrit/js/blob/main/.grit/patterns/importing.md)

Yes, that is exactly what I was looking for. Thanks :)
Currently, the standard library section in the docs does not mention ensure_import_from. It appears to be its own separate section under Guides. Should we move it to the stdlib documentation?

from js.

/bounty $400

from js.

💎 $400 bounty created by morgante
🙋 If you start working on this, comment /attempt #193 to notify everyone
👉 To claim this bounty, submit a pull request that includes the text /claim #193 somewhere in its body
📝 Before proceeding, please make sure you can receive payouts in your country
💵 Payment arrives in your account 2-5 days after the bounty is rewarded
💯 You keep 100% of the bounty award
🙏 Thank you for contributing to getgrit/js!

from js.

/tip $400 @srijan-paul

Solved already.

from js.

👉 @morgante: Click here to proceed

from js.