uuidjs/uuid

Have validate accept null and undefined

grosch opened this issue · 13 comments

grosch commented

Just to make life easier, it would be great if when I call uuidValidate and pass in null or undefined, it returned false. Right now the compiler doesn't allow a potentially null/undefined value to be given.

Right now I'm doing something like this:

this.route.paramMap
    .pipe(
        map(x => x.get('id')),
        filter(x => !!x && uuidValidate(x)),
        mustHaveValue(),
        switchMap(x => this.clientService.get(x)),
        untilDestroyed(this)
    )
broofa commented

Thanks for the report. This is actually a @types/uuid issue. The validate() implementation actually works just fine with any value.

(I'm actually surprised nobody has complained about this yet.)

> require('uuid').validate(null)
false
> require('uuid').validate(undefined)
false

@grosch: We'll fix this up on our end, but this taps into a bigger debate we've been having about whether we should provide TS types natively as pat of this project. In the meantime, I suggest just casting the argument to a string, ala filter(x => uuidValidate(x as string)).

@ctavan Looks like the validate() types were introduced here. I'd put up a PR but I'm wondering if it wouldn't be simpler to pull type definitions into this project.

  • 👍1
ctavan commented

I think it could make sense to fix that on @types/uuid as it's a clear bug and we don't have a timeline for inlining the type definitions into this library.

  • 👍1
ctavan commented

@grosch feel free to propose a pull request with a fix to https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/uuid/index.d.ts

I'll be more than happy to help get this merged quickly over there.

LinusU commented

If you are sending a PR, I would recommend to change it to from (uuid: string) => boolean to (uuid: any) => uuid is string. That way TypeScript will know that you can pass anything in to the validate function, and that if true was returned it was definitely a string.

broofa commented

@LinusU Can you provide a link to the typescript docs that discuss whatever that behavior is that you're describing? I'm still learning TS and this sort of conditional typing sounds interesting.

github-actions commented

Marking as stale due to 90 days with no activity.

github-actions commented

Closing issue due to 30 days since being marked as stale.

ctavan commented

Reopening as this may be solved in #654.

  • 👍2
github-actions commented

Marking as stale due to 90 days with no activity.

github-actions commented

Closing issue due to 30 days since being marked as stale.