luxon
luxon copied to clipboard
Type for validated DateTime
Is your feature request related to a problem? Please describe.
With @types/luxon
in version 3.3.0
it checks if the DateTime may be invalid, forcing to check if the result isn't null
const dateInJson = DateTime
.local() // DateTime
.startOf('day') // DateTime
.plus({months: 3}) // DateTime
.toJSON() // string | null
Describe the solution you'd like
A method like .validated()
that either throw exception or return Validated<DateTime>
skipping the possibility of null
at .toJSON()
const dateInJson = DateTime
.local() // DateTime
.startOf('day') // DateTime
.plus({months: 3}) // DateTime
.validated() // Validated<DateTime> or throws exception
.toJSON() // string
I'm not sure if .local
shouldn't be already valid, startOf
and plus
shouldn't change it, therefore .validated
shouldn't be even required here.
+1 to this - the compiler should be informed that DateTime.now().toISO()
will always be string, because .now()
should always be valid.
Looks like a throwOnInvalid
type settings has been introduced in version 3.3.0
(see PR). As documented on the PR, the type issue can be fixed by overriding the throwOnInvalid
type setting like:
// luxon.d.ts
import 'luxon';
declare module 'luxon' {
interface TSSettings {
throwOnInvalid: true;
}
}
And setup Luxon to throw error on invalid dates as documented here.
I think the behavior @Razi91 has suggested would be an extension of throwOnInvalid
- a way to ensure validity selectively rather than across the entire library. For example, there are some situations where you want to ensure validity absolutely (e.g. if loading a serialized date from a database) and some where you can tolerate failure (e.g. when accepting user input). As a workaround for now, I'll enable the throwOnInvalid
behavior and use try/catch
but being able to do this imperatively would feel more natural to me in TS.
As a side note, it would be great if the required snippets for enabling throwOnInvalid
in both the code and typing were included in the documentation.
I also stumbled upon this issue after recent updates to typings. However I would like to suggest a different solution to the luxon library itself.
Instead of a global setting that one must not forget to set in every importing module luxon could export a "strict" version of API that will have throwOnInvalid = true
. That way developer can chose which version to import and it will always be correctly typed.
I think this is better since it doesn't pollute API with extra .validated()
method that I would need to call on every construction of luxon dates. It also allows for finer choice of where to use strict mode and where not, unlike a global nature of static settings (can lead to subtle bugs when set from different places) and global type override.
I like the idea @shishkin, since yes there is risk that the Settings value gets set or unset somewhere (potentially even within a dependency!) and introduces bugs. But I would suggest you move it to its own issue so it doesn't get missed by the maintaining team. I'd say it's solving a distinct enough problem from the one we're looking at that both should stand in their own right and the library would be better if the team was able to incorporate both, not just one or the other.
@zackdotcomputer you're right. Just created #1421.
Here for the following new type issue:
const test: string = DateTime.now().toISODate()
// Type 'string | null' is not assignable to type 'string'.⏎ Type 'null' is not assignable to type 'string'.
How exactly are we supposed to be handling this now? Is there some IsValid
helper we can utilize?
Documentation still shows that toISODate
should always return a string.
Edit
After playing with this, I guess I need to handle nullability by adding a check and handling it, or defaulting it to some value.
const testOrNull: string | null = DateTime.now().toISODate()
if (testOrNull === null) {
// do something; return or throw
}
const test: string = testOrNull // this will be valid now that we checked for the null scenario
For this it might be nice to utilize something like Results
from https://github.com/vultix/ts-results so that you can denote success or failure explicitly, while having code that does not throw, and leaving it up to the consumer to handle the result type.
Just ran into this tonight while trying to bump from v2.5 to v3.3 and have to say its very unexpected and is a major impact. We use Prisma and because of the introduction of this possible null result literally every place to convert a DateTime to an ISO string is now busted. Frustrating to say the least especially considering .now() should always be valid.
I can't make any runtime changes, but I've improved the types: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/65078
DateTime.now().toISO(); // string
const input = DateTime.fromISO('...');
input.toISO(); // string | null
if (input.isValid) {
input.toISO(); // string
input.invalidReason; // null
} else {
input.toISO(); // null
input.invalidReason; // string
}
This is powered by a validity boolean generic. The default value is either valid or invalid (unless throwOnInvalid
is enabled). To communicate a date was previously validated you can provide this value explicitly (true
for valid, and false
for invalid).
const now = DateTime.now();
function printDate(date: DateTime<true>): string {
return date.toISO();
}
printDate(now);
After playing with this, I guess I need to handle nullability by adding a check and handling it, or defaulting it to some value.
const testOrNull: string | null = DateTime.now().toISODate() if (testOrNull === null) { // do something; return or throw } const test: string = testOrNull // this will be valid now that we checked for the null scenario
I found a much simpler way to deal with this breaking change for my cases was to use the nullish coalescing operator, like this:
const myString: string = myDate.toISODate() ?? '';
@Andrew-Marks-Trisept that handles the typing, but likely you will not want to surface an empty string, most likely you'd want to handle it as an error or something rather than defaulting, in which case you'll end up with a typeguard/type narrowing instead.
@CarsonF I think some of the type signatures are wrong in https://github.com/DefinitelyTyped/DefinitelyTyped/pull/65078
At least for interval
/**
* Returns an error code if this Interval is invalid, or null if the Interval is valid
*/
- get invalidReason(): string | null;
+ get invalidReason(): IfValid<string, null, IsValid>;
/**
* Returns an explanation of why this Interval became invalid, or null if the Interval is valid
*/
- get invalidExplanation(): string | null;
+ get invalidExplanation(): IfValid<string | null, null, IsValid>;
I believe these should be: IfValid<null, string, IsValid>
. Do you agree?
I haven't looked further beyond that, but just noticed this error pop up in my code when trying to narrow types:
if (interval.isValid) {
// do stuff
} else {
const error: string = interval.invalidExplanation; // Type 'null' is not assignable to type 'string'.
}
@BrentSouza Yes, you're right. My bad. Looks like DateTime
was fixed in DefinitelyTyped/DefinitelyTyped#67756. Do you want to PR a fix?
Sure. I'll open a PR shortly