ianstormtaylor / superstruct

A simple and composable way to validate data in JavaScript (and TypeScript).
https://docs.superstructjs.org
MIT License
6.96k stars 223 forks source link

Async Validation Support #1137

Open MentalGear opened 1 year ago

MentalGear commented 1 year ago

I like superstruct's single SOT approach. Coming from Vest, I'm wondering if async validations can be done?

For example, check if a username is available via an API route.

ianstormtaylor commented 1 year ago

@MentalGear I'd recommend separating out the username check from the other validation logic. In my experience coupling things to make them async makes it much more complicated.

MentalGear commented 1 year ago

Hey Ian, thanks for your reply. Could you maybe specify what issues you're referring to? Async with await is rather fair nowadays.

Also, as mentioned, having a single source of truth is what's really attractive, and what I think is also Superstruct's aim.

ianstormtaylor commented 1 year ago

@MentalGear async functions often force everything up the stack to be async too, but we'd need a way to make that not the case because many current use cases rely on logic being synchronous (if statement, TypeScript guards, argument assertions, etc.)

Async also opens a whole new can of worms — loading states, cancellation of extraneous requests, serial vs. parallel, etc., etc. So the system would need to be simple enough that Superstruct isn't required to start expanding its scope a ton to make async validation useful.

If you (or anyone else) wants to try coming up with some designs I'm open to reading them though.

MentalGear commented 1 year ago

How's about adding async tests to the end of the stack and only run if all sync tests succeed?

Taking a note from vest (which also only evaluates changed properties, not the whole test suite):

/**
 * Registers test, if async - adds to pending array
 */
function registerTest(testObject) {
    var bus = useBus();
    // Run test callback.
    // If a promise is returned, set as async and
    // Move to pending list.
    var result = runSyncTest(testObject);
    try {
        // try catch for safe property access
        // in case object is an enforce chain
        if (vestUtils.isPromise(result)) {
            testObject.asyncTest = result;
            testObject.setPending();
            runAsyncTest(testObject);
        }
        else {
            bus.emit(Events.TEST_COMPLETED, testObject);
        }
    }
    catch (e) {
        throw new Error("Unexpected error encountered during test registration.\n      Test Object: ".concat(JSON.stringify(testObject), ".\n      Error: ").concat(e, "."));
    }
}
houd1ni commented 1 year ago

@ianstormtaylor Just a use case: I'm using https://github.com/teomantuncer/node-email-check this is a email checker that asynchronously goes to DNS to check MX record of a domain in email. That's quite useful and, for sure, must be async. For now I have to skip MX validation.