jquense / yup

Dead simple Object schema validation
MIT License
22.94k stars 935 forks source link

addMethod(Schema) not working in TypeScript files #2068

Closed csantos1113 closed 11 months ago

csantos1113 commented 1 year ago

FROM the docs https://github.com/jquense/yup#addmethodschematype-schema-name-string-method--schema-void

If you want to add a method to ALL schema types, extend the abstract base class: Schema

import { addMethod, Schema } from 'yup';

addMethod(Schema, 'myMethod', ...)

It doesn't work in .ts files


Originally posted by @csantos-nydig in https://github.com/jquense/yup/issues/2058#issuecomment-1638215239

I'm not sure you looked at the codesandbox.io/s/mixed-yup1-0-2-forked-l56m8r, but let me add an image here to show you where the TypeScript errors shows up:

Screenshot 2023-07-17 at 8 35 04 AM

I also cloned yup repository locally, and pasted in my code in one of the ts files: the typescript error is the same

image

I see you have unit tests for addMethod https://github.com/jquense/yup/blob/master/test/yup.js#L206-L211, but that file is written in JavaScript, so the TypeScript error doesn't show up

but the TS error appears if you move this simple line from .js to .ts:

addMethod(Schema, 'foo', () => 'here');
image

I don't think it has anything to do with the declare statement, which is actually working: see the green box in the screenshot above, and it also works fine when executed.

image

So the TS error is when calling addMethod on Schema not when using the added method. 👇

image
StevenVerbiest commented 1 year ago

You are using the Schema type as a value?

If you want to add your method to the string schema, you can do this:

declare module "yup" {
  interface StringSchema {
    forAll(): StringSchema;
  }
}

addMethod(string, "forAll", function forAll() {
  return this.transform(() => {
    return "just for testing";
  });
});

const stringSchema = string().forAll();

And if you want to add it to the mixed schema:

declare module "yup" {
  interface MixedSchema {
    forAll(): MixedSchema;
  }
}

addMethod(mixed, "forAll", function forAll() {
  return this.transform(() => {
    return "just for testing";
  });
});

const mixedSchema = mixed().forAll();
csantos1113 commented 1 year ago

@StevenVerbiest thanks for your reply. No, from the docs I want to add a method to ALL types:

https://github.com/jquense/yup#addmethodschematype-schema-name-string-method--schema-void

If you want to add a method to ALL schema types, extend the abstract base class: Schema

Before v1, all schema types extended from mixed, so adding it to mixed added it to all schema types. That is not longer the case and based on the docs we should add it to Schema.

And it "works": the method is available for all schema types. but the addMethod() call throws TS errors when put in Typescript files

csantos1113 commented 11 months ago

This is fixed in v1.3.3 via f921fe69a2d6ecc6e7d0101d2bd81148dfe83e64