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

Add method helper #202

Closed
wants to merge 7 commits into from
Closed

Add method helper #202

wants to merge 7 commits into from

Conversation

mmkal
Copy link
Contributor

@mmkal mmkal commented Jan 24, 2021

This is something I've wished existed for a while. The problem being: I'd like to write a function with specific input types, which is going to be invoked from call-sites outside the project I'm working in (e.g., a library function, an http request/serverless function handler/some json file processor etc.).

But I'd also like to be able to call it internally - the simplest use case being from a unit test.

Example:

// calculator.ts
import ow from 'ow'

const add = ow.method([ow.number, ow.number], (a, b) => a + b)

const subtract = ow.method([ow.number, ow.number], (a, b) => a - b)

const multiply = ow.method([ow.number, ow.number], (a, b) => a * b)

const divide = ow.method([ow.number, ow.number.not.equal(0)], (a, b) => a / b)

Note that a and b are strongly typed (as numbers in the above example) in each function.

Previously the options aren't that great. You can either be safe by typing all inputs as unknown, which makes sure that the validation is done, but offers no help to callers or the function:

const add = (a: unknown, b: unknown) => {
  // good: the type system makes sure we remember to validate the inputs (albeit with some boilerplate)
  ow(a, ow.number)
  ow(b, ow.number)
  return a + b
}

// bad: the type system doesn't stop callers from sending bad inputs.
// This isn't a bug in the function, but it'll still lead to errors:
add('foo', { bar: true })

Or you can type them as number, which provides types to callers, but means that allows skipping the assertion at the type level, leading to errors at runtime if bad data is passed to the function:

// bad: type system has been told the inputs will definitely be a number. But what if the inputs come from an `any` type, or javascript?
const add = (a: number, b: number) => a + b

app.use((req, res) => {
  // undefined behaviour if someone sends a body like { "a": "foo", "b": { "bar": true } }?
  res.send({ sum: add(req.body.a, req.body.b) })
})

Or, you can write a separate type signature, leading to confusing/unnecessary boilerplate:

Instead of:

const add = ow.method([ow.number, ow.number], (a, b) => a + b)

you would have to do:

const add: (a: number, b: number) => number = (a: unknown, b: unknown) => {
  ow(a, ow.number)
  ow(b, ow.number)
  return a + b
}

Curious to hear any thoughts on the idea!

Update :I've since noticed similar functionality in zod.

Base automatically changed from master to main January 24, 2021 06:14
| [T, T, T, T, T]
| [T, T, T, T, T, T]
| [T, T, T, T, T, T, T]
| [T, T, T, T, T, T, T, T];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's common to support 10 when doing this kind of thing.

@@ -119,6 +119,20 @@ checkPassword('foo');
//=> ArgumentError: Expected string `password` to have a minimum length of `6`, got `foo`
```

### ow.method(predicates, label, body)

Wrap a function with parameter validation. Useful for writing strongly-typed functions which will be called with untrusted input.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be useful to elaborate a little bit more on the use-case. As a user, I'm not entirely sure still when I would use it.

*/
method<Arguments extends Tuple<any>, Return>(
predicates: Extract<{[K in keyof Arguments]: BasePredicate<Arguments[K]>}, any[]>,
functionName: string,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need an overload here? Can't we just make this parameter optional?

@sindresorhus
Copy link
Owner

Looks like a useful utility to me.

@sindresorhus
Copy link
Owner

What do you think about the name .wrap instead of .method?

@sindresorhus
Copy link
Owner

Bump :)

@mmkal
Copy link
Contributor Author

mmkal commented Apr 25, 2021

@sindresorhus sorry, I don't think I'll find the time anytime soon to take this over the line, so closing. Feel free to use as a reference for a future implementation though/if you want to make an issue for this idea.

@mmkal mmkal closed this Apr 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants