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

How to map moment to number? #60

Closed
krukru opened this issue Jan 28, 2020 · 11 comments
Closed

How to map moment to number? #60

krukru opened this issue Jan 28, 2020 · 11 comments
Labels
awaiting reply question Further information is requested

Comments

@krukru
Copy link

krukru commented Jan 28, 2020

Hi again :)

This time I am wondering how to map between moment object and number

Here is my snippet

import "reflect-metadata";
import { AutoMap, Mapper } from "@nartc/automapper";
var moment = require("moment");

class Foo {
  @AutoMap()
  public get foo(): Moment {
    return this._foo;
  }

  public set foo(value: Moment) {
    this._foo = value;
  }

  private _foo!: Moment;
}

class FooDTO {
  @AutoMap()
  public foo!: number; // the unix timestamp
}

Mapper.initialize(cfg => {
  cfg.createMap(FooDTO, Foo).forMember(
    (dest: Foo) => dest.foo,
    (opts: any) => opts.mapFrom((source) => {
      return moment(source.foo);
    }),
);
});

const foo = Mapper.map({ foo: 1111234567890}, Foo, FooDTO);
console.log(foo);

This fails with the error "TypeError Cannot read property '_isAMomentObject' of undefined".
The issue is related to class-transformer (see typestack/class-transformer#176) but I don't know how to get this to work using your mapper lib.

@nartc
Copy link
Owner

nartc commented Jan 28, 2020

Hi, thanks for reporting the issue.

I thought about Moment object but ended up not supporting it since it would mean I would need to support another 3rd party library and there are many different choices regarding Date now. If I could suggest, try: date-fns, dayjs or luxon.

However, if Moment is crucial for your project, I’ll see what I can do.

@nartc nartc added awaiting reply question Further information is requested labels Jan 28, 2020
@krukru
Copy link
Author

krukru commented Jan 28, 2020

Hey, it is not crucial :)
I don't really like moment due to the way it is written, but decided to go with it since it seemed most used/popular. Maybe it is time to reconsider :)
Thanks anyway, I am closing the issue (and maybe it would be worth noting that moment is not supported)

@krukru krukru closed this as completed Jan 28, 2020
@nartc
Copy link
Owner

nartc commented Jan 28, 2020

@krukru Though it is the most popular one, it’s a heavy library.

  • Luxon: is written using Moment’s concept but is redesigned to be lighter and less-clunky than Moment, written by one of the old Moment’s maintainer
  • date-fns: like Lodash for Date
  • dayjs: extremely lightweight with plugin to enable various features.

I’ve used date-fns and dayjs before and they’re great. Working with native JS Date object is awesome with those libs. I would definitely reconsider and will write a small section regarding DateTime on the Readme. Thanks again

@nartc
Copy link
Owner

nartc commented Jan 28, 2020

@krukru I found a workaround for Moment object. Read the Date Time section on the README to learn about it. Though, I would still suggest going with vanilla JS Date object if you can.

@krukru
Copy link
Author

krukru commented Jan 28, 2020

Hey, thanks for updating the docs, I almost got this to work after updating to 3.1.2 :)
Here is what I am working with

import "reflect-metadata";
import { AutoMap, Mapper } from "@nartc/automapper";

import { Moment } from "moment";
const moment = require("moment");

class Foo {
  @AutoMap(() => moment)
  public get foo(): Moment {
    return this._foo;
  }

  public set foo(value: Moment) {
    this._foo = value;
  }

  private _foo!: Moment;
}

class FooDTO {
  @AutoMap()
  public foo!: number; // the unix timestamp
}

Mapper.initialize(cfg => {
  cfg.createMap(FooDTO, Foo).forMember(
    (dest: Foo) => dest.foo,
    (opts: any) => opts.mapFrom((source) => moment(source.foo)),
  ).reverseMap().forPath(
    (dest: FooDTO) => dest.foo,
    (opts: any) => opts.mapFrom((source) => source.foo.unix()))
});

const fooDomainObjectMapped = Mapper.map({ foo: 12345 * 1000}, Foo, FooDTO);
expect(fooDomainObjectMapped.foo.unix()).equals(12345); // this passes as true!

const fooDomainObject = new Foo();
fooDomainObject.foo = moment(54321 * 1000);

const fooDtoObjectMapped = Mapper.map(fooDomainObject, FooDTO);
expect(fooDtoObjectMapped.foo).equals(54321); // this fails, fooDtoObjectMapped.foo is equal to the current system timestamp

So mapping from Dto(Vm) -> Domain model works
But mapping Domain model -> Dto fails and always creates a moment object with the current timestamp. Now that's really odd, isn't it! :)

@krukru
Copy link
Author

krukru commented Jan 28, 2020

What's maybe more interesting is how this snippet works on 3.1.0

const fooDomainObjectMapped = Mapper.map({ foo: 12345 * 1000}, Foo, FooDTO);
expect(fooDomainObjectMapped.foo.unix()).equals(12345); // this fails, fooDomainObjectMapped.foo.unix() is equal to the current system timestamp
const fooDtoObjectMapped = Mapper.map(fooDomainObject, FooDTO);
expect(fooDtoObjectMapped.foo).equals(54321); // this throws a runtime error: TypeError: Cannot read property '_isAMomentObject' of undefined

I am trying to deduce from the source and commits between versions what has changed, but it is a bit over my head. Hope this piece of info helps!

@nartc
Copy link
Owner

nartc commented Jan 28, 2020

hmmm I'll give that a quick run on my test bed.

@nartc
Copy link
Owner

nartc commented Jan 28, 2020

@krukru plainToClass from class-transformer initializes a new Moment object under the hood. That's a bummer. I don't think Moment is working with this library as of the moment.

@nartc
Copy link
Owner

nartc commented Jan 28, 2020

@krukru Another point is I run plainToClass against the source (that was passed in Mapper.map(source, DestinationModel)) every time. I just modified the code to where plainToClass is only ran if source isn't a true instance of mapping.source (aka if source is a plain object). If source is a true instance of mapping.source, then it seems like moment is working.

Let me know (if you're still interested) if the latest version works out for you.

@krukru
Copy link
Author

krukru commented Jan 29, 2020

I don't know what you did, but in 3.1.2 this now works in both directions!
Thanks very much for all the support, you are awesome :)

@nartc
Copy link
Owner

nartc commented Jan 29, 2020

@krukru I simply changed the way Automapper interacts with class-transformer a bit. Thanks for reporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting reply question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants