introduced model interface IPerson

[marcwittke]

Hi @harblaith7

I just finished your tutorial about react and typescript. Well done, I think I got the point. However, one issue left me puzzled. As someone with a strong background in typed languages (say: 20 years of Java, C#) I am still trying to figure out why you are poking around with exporting the IState as a whole, importing it under a different name and then accessing the nested type on it in several components. IMHO this introduces unnecesary coupling and is an accident waiting to happen (quite similar to what uncle Bob calls "Train Wrecks")

For me it would be the most obvious to define the reappearing model type strictly and use it in different "envelopes", like IProps and IState. This type would also be the location of shared business rules like "age must be greater than 21" or something like that.

is there a good reason not to do it like in this PR?

[PhilippSpo]

We should avoid type assertions if we can. It basically tells typescript "I know more than you do about this type", with the risk of being wrong 😄 .

We shouldn't need to tell typescript about the type of this object, as the setPeople handler only accepts People to be set.

[marcwittke]

as a dotnetter I always shudder when I see as, too. But, here it is the creation of an instance - is there a better way to make an instance of an interface?

[PhilippSpo]

There is not really such thing in typescript due to structural typing. If an object has the same shape as the IPerson interface you can consider it an instance of it.

But when you create a variable and want it to explicitly be of type IPerson you can declare it like so:

const person: IPerson = {
    name: "Marc",
    age: 1,
    img: "foo"
}

(playground example)

[PhilippSpo]

I would agree, the more we are using this type across our codebase, the more it makes sense to extract it to a separate location.

I totally agree with the coupling argument. The AddToList component should not be coupled to be used inside the List component. It should not care where it is used.

The question is rather, should the AddToList component be coupled to the general purpose IPerson interface? Or should it define its own type?

One could argue, that as a presentational component, it defines its own type. It renders four input fields no matter how the IPerson interface changes. Then if the IPerson interface changes, we wouldn't necessarily need to change the source code of the AddToList component, but could change how we pass the props to it in the List.

On the other hand, one could say that in this specific app, it makes sense that whenever we change the IPerson interface, the AddToList component should change with it.

I hope this made sense 😅

[PhilippSpo]

Using React.Dispatch<React.SetStateAction<>> already introduces coupling as we are basically saying that it must be a native react state setter the needs to be passed in here. But what if we are not using react state in the parent component?

We could, instead, define what the AddToList component actually expects: A function that takes a list of persons and returns nothing:

Suggested change

	setPeople: React.Dispatch<React.SetStateAction<IPerson[]>>
	setPeople: (persons: IPerson[]) => void