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

isValidPhoneNumber returns different results for same number depending on where it is called #470

Open
glassworks-projects opened this issue Sep 4, 2024 · 2 comments

Comments

@glassworks-projects
Copy link

A bug so bizarre that it must be because I'm doing something simple wrong, but here goes:

I have a full stack react app in which phone numbers can be added to a database using a form or via CSV upload. Here's my validation schema:

import { needType } from "@/schema";
import { isValidPhoneNumber } from "libphonenumber-js";
import { z } from "zod";

export const voterNeedSchema = z.object({
  name: z.string().max(256),
  address: z.string().max(256),
  phone: z
    .string()
    .refine(isValidPhoneNumber, { message: "Invalid phone number" }),
  county: z.string().max(64),
  type: z.enum(needType.enumValues),
  note: z.string(),
});

On the frontend, I've got a react-hook-form using the schema:

// routes/addVoterNeed.tsx
import { useForm } from "react-hook-form";
import { zodResolver } from "@hookform/resolvers/zod";
import { z } from "zod";
type formType = z.infer<typeof voterNeedSchema>;

const resolver = zodResolver(voterNeedSchema);

export default function AddVoterNeed() {
  const form = useForm<formType>({
    resolver,
    defaultValues: {
      name: "",
      address: "",
      phone: "",
      county: "",
      type: "Unknown",
      note: "",
    },
  });
... 

And then I'm using a <PhoneInput /> element from react-phone-number-input to collect the number.

For the CSV upload, I am passing the value as text to a tRPC mutation because tRPC doesn't handle multipart/form-data requests. (Maybe this is the problem?)

// components/needsUpload.tsx
import { Button } from "@/components/ui/button";
import { Form, FormControl, FormField, FormItem } from "@/components/ui/form";
import { Input } from "@/components/ui/input";
import { csvSchema } from "@/utils/schemas";
import { trpc } from "@/utils/trpc";
import { zodResolver } from "@hookform/resolvers/zod";
import { UploadIcon } from "lucide-react";
import { ChangeEvent, useRef } from "react";
import { useForm } from "react-hook-form";
import { z } from "zod";

type formType = z.infer<typeof csvSchema>;

const resolver = zodResolver(csvSchema);

export const NeedsUpload = () => {
  const mutation = trpc.uploadVoterNeeds.useMutation();
  const fileInputRef = useRef<HTMLInputElement>(null);
  const form = useForm<formType>({
    resolver,
  });

  const onSubmit = async (values: formType) => {
    const text = await values.file.text();
    mutation.mutate(text);
  };

  const handleFileChange = (e: ChangeEvent<HTMLInputElement>) => {
    if (e.target.files) {
      const file = e.target.files[0];
      form.setValue("file", file);
      form.handleSubmit(onSubmit)();
    }
  };

  return (
    <>
      <Button
        variant="secondary"
        className="flex gap-1"
        onClick={() => fileInputRef.current?.click()}
      >
        <UploadIcon size={18} />
        Upload
      </Button>
      <Form {...form}>
        <FormField
          name="file"
          control={form.control}
          // eslint-disable-next-line @typescript-eslint/no-unused-vars
          render={({ field: { value, onChange, ref, ...rest } }) => (
            <FormItem hidden>
              <FormControl>
                <Input
                  type="file"
                  accept="text/csv"
                  onChange={handleFileChange}
                  ref={fileInputRef}
                  {...rest}
                />
              </FormControl>
            </FormItem>
          )}
        />
      </Form>
    </>
  );
};
// router.ts

const t = initTRPC.context<Context>().create({
  transformer: superjson,
});
...
export const appRouter = t.router({
  uploadVoterNeeds: protectedProcedure
    .input(z.string())
    .mutation(async (opts) => {
      const { input } = opts;
      const x = parseCsv(input);
      return null; // obviously I'm going to do more here, but I haven't gotten that far yet because of this bug
    }),
...
})
// utils/parseCsv.ts

import { parse } from "csv-parse/sync";
import { voterNeedSchema } from "./schemas";
import { z } from "zod";

const insertSchema = z.array(voterNeedSchema);

export const parseCsv = (input: string) => {
  const records = parse(input, {
    columns: true,
  });

  const parsed = insertSchema.parse(records);

  return parsed;
};

Tried to be as exhaustive as I could here, but omitted some things for brevity, lmk if you need clarification on anything.

With this setup, when I pass a valid US phone number (a real phone number taken from my contacts) via the form, the validation passes. When I pass that same phone number in a CSV, validation fails. Anyone got any ideas for me? the LLMs have failed me, and I can't really figure out how to debug further. My best guess is that it has something to do with how I'm passing data to the tRPC route, but I can't rule out that it's an issue with the validator function itself, so I'm posting here.

I did try manually testing one of the records in parseCsv to confirm that the check really is returning false, and it is:

  const testPhone = parsed[0].phone;

  console.log(
    `Phone number ${testPhone} valid: ${isValidPhoneNumber(testPhone)}`, // returns false
  );

Note: I was originally importing isValidPhoneNumber from "react-phone-number-input", but since that package just exports that function from this one, I'm posting here. I changed the import (as you can see above) and confirmed the same behavior.

@catamphetamine
Copy link
Owner

Hi. I can see that you've described your issue in much detail but that's also a "cons" because I personally won't bother reading though that. The bottom line in general is: library maintainers can only look at minimal reproducible sandboxes, and that usually means plain javascript + the library as the only included dependency.

@glassworks-projects
Copy link
Author

Hi. I can see that you've described your issue in much detail but that's also a "cons" because I personally won't bother reading though that. The bottom line in general is: library maintainers can only look at minimal reproducible sandboxes, and that usually means plain javascript + the library as the only included dependency.

super unhelpful, thanks so much 😇

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

No branches or pull requests

2 participants