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

Cast from an async sanitizer #42

Open
getlarge opened this issue Oct 30, 2020 · 9 comments
Open

Cast from an async sanitizer #42

getlarge opened this issue Oct 30, 2020 · 9 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@getlarge
Copy link

getlarge commented Oct 30, 2020

It seems it is not yet possible to use an async sanitizer function inside cast, do i mistake ?

If not, would it be possible to add support for this ?

Example

Sanitizer

import { Result } from '@restless/sanitizers';

export const asCertificate = async (value, path: string) => {
  const errors = (await validate(value)) as any[];
  
  if (Object.keys(errors).length) {
    const resultError = flatten(
      Object.values(errors).map((error) => {
        return error.map((error) => ({
          path: `${path}${error.path}`,
          expected: error.expected || '',
        }));
      })
    );

    console.log(resultError);
    return Result.error(resultError);
  }
  return Result.ok(value as Certificate);
};

Cast

import {cast} from '@restless/sanitizers';
import {asCertificate} from './somewhere';

const certificate = {};
const value = cast(certificate, asCertificate);
@sz-piotr
Copy link
Contributor

Yes that is correct. Currently there is no notion of an async sanitizer in the library, but a PR would be greatly appreciated. In the meantime all you really need to do is await the result of asCertificate and then unwrap it like so: https://github.com/EthWorks/restless/blob/master/sanitizers/src/cast.ts#L11-L16

@sz-piotr sz-piotr added good first issue Good for newcomers help wanted Extra attention is needed labels Oct 30, 2020
@getlarge
Copy link
Author

I'm ok to make a PR. Would that be fine to add an asyncCast function that takes a Promise<Sanitizer<T>> as input and returns Promise<T> ?

@sz-piotr
Copy link
Contributor

That could also work, but it does not cover your use case. Your use case is a sanitizer that returns a promise, not a promise with a sanitizer.

@sz-piotr
Copy link
Contributor

sz-piotr commented Nov 2, 2020

Hey @getlarge. I thought about your issue over the weekend and here is what I came up with:

import { Result, SanitizerFailure, CastError } from "@restless/sanitizers";

export type AsyncSanitizer<T> = (value: unknown, path: string) => Promise<Result<SanitizerFailure[], T>>

export async function asyncCast<T>(value: unknown, sanitizer: AsyncSanitizer<T>, message?: string): Promise<T> {
  const result = await sanitizer(value, '')
  if (Result.isOk(result)) {
    return result.ok
  } else {
    throw new CastError(result.error, message)
  }
}

You should be able to drop this code in your project and use it without issues.

If however you wanted a function that takes a Promise<Sanitizer<T> it becomes even easier:

import { Sanitizer, cast } from "@restless/sanitizers";

export async function asyncCast<T>(value: unknown, sanitizer: Promise<Sanitizer<T>>, message?: string): Promise<T> {
  return cast(value, await sanitizer, message)
}

@getlarge
Copy link
Author

getlarge commented Nov 2, 2020

Hey @sz-piotr, sorry for the delay. You were right, the use case is a sanitizer that returns a promise.

Thanks a lot for your snippet and your feedback.
Do you intend to add the AsyncSanitizer type and asyncCast function in restless ?

@getlarge
Copy link
Author

getlarge commented Nov 2, 2020

In fact my question was more to know if you would prefer a cast function that could take an async | sync sanitizer as 2nd argument or to separate async and sync sanitizer with another function and type.

@sz-piotr
Copy link
Contributor

sz-piotr commented Nov 2, 2020

Here is my line of thinking. If async sanitizers were to be supported by default by the library this should be a bigger change, where all the other higher-order sanitizers like asArray would take either a sync or async sanitizer. In this scenario it makes sense that cast serves a dual purpose being sync or async depending on what it receives.

However since there are currently no async sanitizers in the library code I believe keeping the asyncCast function as separate from both the cast and the library itself makes sense.

@getlarge
Copy link
Author

getlarge commented Nov 3, 2020

But since a user can create an async sanitizer maybe we can add an example or test to illustrate that case ?

@sz-piotr
Copy link
Contributor

sz-piotr commented Nov 3, 2020

Hmm might be indeed useful once we have the notion of AsyncSanitizers somewhere in the library. If in the future we add this it will definitely be described in the readme.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants