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

inSequence doesn't pass the updated input from a validation to the next one #20

Closed
pedrodim opened this issue Dec 14, 2022 · 1 comment · Fixed by #21
Closed

inSequence doesn't pass the updated input from a validation to the next one #20

pedrodim opened this issue Dec 14, 2022 · 1 comment · Fixed by #21

Comments

@pedrodim
Copy link
Member

pedrodim commented Dec 14, 2022

When using inSequence and changing the type of the input value with a validator, the types suggests that the following validator will receive the updated input value but this is not true, as the return of the validator just gets discarded, from what I can see on the implementation and the function type this shouldn't work like this.

validators.inSequence(
  validators.validator<string, number, string>((input) => {
    const newInput = parseFloat(input);
    return Number.isNaN(newInput) ? success(newInput) : failure("Invalid number");
  }),
  validators.fromPredicate((input) => Number.isFinite(input), "Number is not finite"),
)

For an input of "100" I would expect the validation to success, but instead it fails on the second validator as input is "100" (string) instead of 100 (number).

The problem seems to be with this line input = r.success;, I suppose it should be value = r.success; so that the next validation will receive the updated input.

formo/src/validators.ts

Lines 90 to 97 in 5fdd851

let value = input;
for (const validator of validators) {
const r = await validator(value);
if (r.type === "failure") {
return r;
}
input = r.success;
}

@giogonzo
Copy link
Member

@pedrodim indeed it looks like a bug, and I believe your proposed solution is fine. Would you mind submitting a PR? I would also like to add a test for this, such as (or a better one, I made this up in a few minutes):

describe("validators", () => {
    describe("inSequence", () => {
      test("should pass the validation output to the subsequent one", async () => {
        const v1 = validators.validator((a: string) => success(a.length));
        const f = jest.fn((b: number) => success(b));
        const v2 = validators.validator(f);
        await validators.inSequence(v1, v2)("foo");
        expect(f).toHaveBeenCalledWith(3);
      });
    });
  });

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 a pull request may close this issue.

2 participants