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

Default config DateTime constructor #1466

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jmostaer
Copy link

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.

Set default value for config in DateTime constructor
@linux-foundation-easycla
Copy link

CLA Not Signed

@diesieben07
Copy link
Collaborator

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
Copy link
Author

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

@diesieben07
Copy link
Collaborator

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
Copy link
Author

jmostaer commented Jul 13, 2023

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: typestack/class-transformer#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
Copy link
Collaborator

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() }));

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.

2 participants