catamphetamine / libphonenumber-js

A simpler (and smaller) rewrite of Google Android's libphonenumber library in javascript
https://catamphetamine.gitlab.io/libphonenumber-js/
MIT License
2.8k stars 216 forks source link

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

Open glassworks-projects opened 2 months ago

glassworks-projects commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 😇