moment / luxon

⏱ A library for working with dates and times in JS
https://moment.github.io/luxon
MIT License
15.23k stars 730 forks source link

Default config DateTime constructor #1466

Open jmostaer opened 1 year ago

jmostaer commented 1 year ago

When using DateTime with mappers such as class-transfomers, these libraries will try to use the DateTime constructor without parameters. This currently will throw an exception. Setting a default value for config in the constructor solves this problem.

linux-foundation-easycla[bot] commented 1 year ago

CLA Not Signed

diesieben07 commented 1 year ago

The constructor is private API and should not be called by anyone but Luxon itself. Can you clarify which library you are using that requires calling the constructor directly?

jmostaer commented 1 year ago

We are using class-transformer. I think class-transformer uses the constructor to try to create a deep copy of the DateTime instance.

diesieben07 commented 1 year ago

I haven't used class-transformer myself. Looking at their docs, it seems they show how to add manual transformations. This (or similar APIs) should be used to make the library not use the private constructor.

jmostaer commented 1 year ago

We do use transformations. In the class we are tranforming to, the date is not even stored as a luxon DateTime, but as string. Luxon is only used in the plain object we are trying to convert to the class. However, to my understanding, class-transformer will first try to create a deep copy of the object to be transformed before executing the transformations on the object (probably to avoid accidentally altering the original object in the transformations). This is where the constructor of DateTime is used an where the problem above pops up.

This issue is related to this issue on the class-transformer repository: https://github.com/typestack/class-transformer/issues/132 Support for custom constructors is currently not on their roadmap, so there is no solution from their end for now.

We are currently using this workaround in our codebase, but I would very much like to avoid this.



const originalConstructor = DateTime.prototype.constructor;
DateTime.prototype.constructor = function (config = {}) {
    return new originalConstructor(config);
};
diesieben07 commented 1 year ago

I cannot reproduce what you mentioned. As long as class transformer understands that you want a string in your class and not a DateTime, it works just fine:

import 'reflect-metadata';
import {plainToClass, Type} from 'class-transformer';
import {DateTime} from 'luxon';

class Test {
    @Type(() => String)
    dateTime: string;
}

console.log(plainToClass(Test, { dateTime: DateTime.now() }));