Separate export for throwOnInvalid strict mode

[shishkin]

This issue is extracted from https://github.com/moment/luxon/issues/1419#issuecomment-1509608877.

The recent updates to typings made it apparent that I wasn't aware of luxon's default behavior with respect to validity. After reading about throwOnInvalid setting, I would like to suggest to create a separate export for the strict mode of the library.

Instead of a global setting that one must not forget to set in every importing module, luxon could export a "strict" version of the API that will have throwOnInvalid = true. That way developer can chose which version to import and it will always be correctly typed.

That strict API should also ignore runtime changes to the global throwOnInvalid setting to stay true to the API contract (respect typings). Or potentially, global setting throwOnInvalid could be removed altogether to avoid subtle bugs when the setting is "flickering" at runtime changed by various modules.

The usage could look something like this:

import { DateTime } from "luxon/strict";

const dt = DateTime.fromISO("blah"); // throws
const s: string = dt.toISO(); // always string, never null

[ab-pm]

This sounds like a good idea. I'd even make this the default (which, admittedly, would require a major version bump).

I guess we all concur that a global mutable throwOnInvalid setting is not working for a lot of use cases. However, exporting two separate modules (be it luxon and luxon/strict, luxon/sloppy and luxon, or even just { ValidDateTime, AnyDateTime } from 'luxon') has its drawbacks as well. This would require two distinct class objects with different static methods. Should they still share their prototype, so that new DateTime() instanceof ValidDateTime (and vice versa)? Should they be subclasses, with ValidDateTime extends AnyDateTime? (Gladly, being immutable, one actually is a subtype of the other, avoiding the circle-ellipse problem). This might lead to a major refactoring of the JS code base.

The alternative would be to keep only one class, and make the isValid assertion only on the type level. In that case, all of the (parsing?) methods that might create invalid dates would be duplicated, e.g. DateTime.fromISO and DateTime.fromValidISO. Or maybe we'd just make throwOnInvalid into one of the options that can be passed to the parsing methods, so that it can be controlled on a more fine-grained level?

[lkhari]

I think removal of the global throwOnInvalid make sense, a lot of people don't read the whole documentation. Adding on to @ab-pm

A simple solution would be to split everything in to 3 classes, you have your main constructor class(i.e fromISO) and then you have your DateTime class, which has all the methods we use and love, and then we have the InvalidDateTime. Or to be honest the constructor fns can stay in DateTime.

The main constuctor fns will have new functions using the prefix safe similar to zod for "non throw error on invalid" such as

    static function utc(): DateTime // no arguements shouldn't return invalidDateTime
    static function safeUtc(...): DateTime // safeUtc should most like have types for some arguments

    static function fromIso(....): DateTime // this will throw an error
    static function safeFromIso(....): DateTime | InvalidDateTime

DateTime and InvalidDateTime wouldn't share the same functions, since InvalidDateTime is only really needed to get the reason of it's invalidity (from my understanding, use cases could be nice if i'm wrong). And to distingiush between the two classes you can just have a typeGuard fn. Like

class DateTime {
  isValid(): true {
    return true;
  }
}

class InvalidDateTime {
  isValid(): false {
    return false;
  }
}

const isDateTime=(a: DateTime | InvalidDateTime): a is DateTime => a.isValid()

[CarsonF]

I don't like the separate exports approach, like luxon/strict. Then you have two types and its easy to mix them up and hard for your editor to know which one you want.

If it were up to me I'd have

class DateTime {
  static fromX(...): DateTime // or throws error with information
  static maybeFromX(...): DateTime | ErrorInfo;
}

sub maybe for safe or whatever.

I just don't see the use case for carrying invalid data forward. Handle your invalid data right there at conversion with an if statement or a catch statement.

[schmod]

Another possibility would be to just make this available as an option on any methods that could potentially throw (ie. DateTime.fromISO(str, { throwOnInvalid: true }))

It's more verbose, but it'd easily be the least-disruptive change.