markitondemand / moment-msdate Goto Github PK

Using moment-msdate in place of moment or moment-timezone in a TypeScript project requires type definitions for its methods. Moment + Timezone are already typed.

I'm still pretty new to TypeScript, but I may create a PR we can review and hopefully get this in.

All it may involve is (add anything I'm missing)

• Create TS def file moment-msdate.d.ts
• Link to package.json on typings key

I'm ok with the idea of a moment plugin for OADates, but I have a few concerns:

• I would expect the external API to be moment.fromOADate(x), not moment().fromOADate(x). By attaching it to the moment instance, you're first creating a "now" moment, then throwing it away.
• The function clearly returns a moment, otherwise it wouldn't be in here at all. So in the docs, I wouldn't show passing the result out as a jsDate and back into the moment constructor again.
• You might want to mention that an OADate is "unspecified", and therefore based on the local time zone by default. You might also want a clear way to pass in an OADate that is based on UTC.
• In the implementation itself, you appear to be taking the timezone from a UTC-based conversion and then applying it to the input to determine the local time. In other words Offset(UTC(OADate)) + UTC(OADate). That's not necessarily going to yield the correct value. The offset applies at the destination - not at the source. Instead, you should be starting with a new Date of the OADate base and working from there.
• There are some implementation quirks about negative numbers that are described in the MSDN reference, but you don't appear to be handling them. You might want to consider the internal .Net implementation code of DateTime.FromOADate which is as follows:

// Creates a DateTime from an OLE Automation Date. 
//
public static DateTime FromOADate(double d) { 
    return new DateTime(DoubleDateToTicks(d), DateTimeKind.Unspecified); 
}

// Converts an OLE Date to a tick count. 
// This function is duplicated in COMDateTime.cpp 
internal static long DoubleDateToTicks(double value) {
    if (value >= OADateMaxAsDouble || value <= OADateMinAsDouble) 
        throw new ArgumentException(Environment.GetResourceString("Arg_OleAutDateInvalid"));
    long millis = (long)(value * MillisPerDay + (value >= 0? 0.5: -0.5));
    // The interesting thing here is when you have a value like 12.5 it all positive 12 days and 12 hours from 01/01/1899
    // However if you a value of -12.25 it is minus 12 days but still positive 6 hours, almost as though you meant -11.75 all negative 
    // This line below fixes up the millis in the negative case
    if (millis < 0) { 
        millis -= (millis % MillisPerDay) * 2; 
    }

    millis += DoubleDateOffset / TicksPerMillisecond;

    if (millis < 0 || millis >= MaxMillis) throw new ArgumentException(Environment.GetResourceString("Arg_OleAutDateScale"));
    return millis * TicksPerMillisecond; 
}

While it's not going to be identical in JavaScript, your code should follow the same logic.

• You might also want to implement .toOADate()
• Unit Tests?

This plugin should be installable via npm install moment-msdate like all other plugins...

Build seems to be failing because of es6 fat arrow.

Documentation should be updated to instruct users to run npm install --save moment-msdate. Also, see about momentjs updating their documentation located here: http://momentjs.com/docs/#/plugins/msdate/