prismicio / prismic-types

Type definitions for Prismic content, models, and APIs
https://prismic.io/docs/technologies/javascript
Apache License 2.0
11 stars 4 forks source link

Input attributes + Typescript #33

Closed kb1995 closed 2 years ago

kb1995 commented 2 years ago

I have an interface:

interface Input {
  type: "input" | "input-email" | "textarea";
  name: KeyTextField;
  label: KeyTextField;
  placeholder: KeyTextField;
  required: boolean
}

I get the following Typescript error for the following attributes

Type 'string | null' is not assignable to type 'string | undefined'.ts(2322)
[index.d.ts(2216, 9): ]()The expected type comes from property 'placeholder' which is declared here on type 'DetailedHTMLProps<InputHTMLAttributes<HTMLInputElement>, HTMLInputElement>'

In order to fix the error, I had to update my Props

interface Input {
  type: "input" | "input-email" | "text-area";
  name: string | undefined;
  label: string | undefined;
  placeholder: string | undefined;
  required: boolean;
}

Is it possible to update the KeyTextField?

lihbr commented 2 years ago

Hey there, I had a look at your issue, here's my understanding:

The placeholder attribute expected by DetailedHTMLProps<InputHTMLAttributes<HTMLInputElement>, HTMLInputElement> has to be of type string | undefined.

Meanwhile, Prismic API returns null | "" for an empty Key Text field, string for a filled Key Text field, therefore the KeyTextField type is typed representing that: https://github.com/prismicio/prismic-types/blob/master/src/fields.ts#L725-L732

Typing the placeholder property of the Input interface as string | undefined feels wrong as the API would never yield undefined for a KeyTextField (which is by design, because undefined is not part of the JSON spec (see ECMA-404, Section 5). The API would also yield null in certain cases which wouldn't be handled by your code.

My suggestion is to conditionally apply the placeholder prop, something along those lines:

/* ... */

return (<>
    <Foo placeholder={input.placeholder ?? undefined} />
    // or, if you wan the HTML to append an empty placeholder instead:
    <Foo placeholder={input.placeholder ?? ""} />
</>);

Let me know if that helps ☺️

angeloashmore commented 2 years ago

Hey @kb1995, just wanted to reiterate that @lihbr's explanation is accurate.

KeyTextField should remain as string | null because the API result will be one of those values, and never undefined.

React expects undefined rather than null to represent an empty placeholder, which is why you see the error. If TypeScript didn't raise an error, I believe you would still see it in the browser console as a React warning.

The only adjustment I would make to @lihbr suggestion is to use || rather than ?? to handle the case of an empty string ("").

<Foo placeholder={input.placeholder || undefined} />
kb1995 commented 2 years ago

Thanks for the explanation - makes sense now!