I would recommend typing this as:

const Header: React.FC<Props> = ({ darkModeController }) => (

Not only is it a few characters shorter, but it also tells that the Header is a React functional component, instead of just being a function that happens to conform to the interface of a functional component ☺️

Id welcome any guidance on typings with React, I dont actually know much about this at all honestly.

What Im seeing here is that because we are using a function expression we should document that expression returns a react functional component function, instead of just documenting the return type of that function itself.

I dont fully understand the benefits of this but I do think I understand the semantic difference.

Thank you for taking a look at this and bringing it up, Im very open to a discussion around the conventions we are using ❤️

Thanks for the feedback @LinusU

Thanks @LinusU… react continues to impress me on how they fuzz TypeScript types :)

So I dug this out — https://unpkg.com/browse/@types/react@16.9.22/index.d.ts#L515:

type FC<P = {}> = FunctionComponent<P>;
interface FunctionComponent<P = {}> {
  (props: PropsWithChildren<P>, context?: any): ReactElement \| null;
  propTypes?: WeakValidationMap<P>;
  contextTypes?: ValidationMap<any>;
  defaultProps?: Partial<P>;
  displayName?: string;
}

Sure enough they wrapped an interface with a type as an alias, and sadly that means it will not type … | (props: P) => JSX.Element | (props: P, context: any) => JSX.Element which is really the developer facing type in all of this — sorry, deep breaths :)

@jonchurch here is a good ref for react typing needs — https://github.com/typescript-cheatsheets/react-typescript-cheatsheet/blob/master/README.md#the-types-i-need-werent-exported

Q

do we want to bother retyping everything?

I honestly would not bother if it is not throwing… typing working for our needs, not React strict but still React.

So if we opt for it we will need to do work all over the place, and that can potentially break things or at least complicate other PRs

Q

do we really need JSX.Element coercion?

I honestly think it is always best to rely on TypeScripts inferred types unless you know for sure there is a glitch or preference… they do a good job nowadays that sometimes it is more accurate than coercion.

So doing it like this meant TypeScript inferred either JSX.Element or React.Element… not sure I can tell which with certainty as of current, but we can check out the code see what hovers.

@benhalverson — I just unresolved this conversation, let's figure out a game plan here then see if we need to create a new PR, push here and merge, or go the longer "task" issue route.

Sorry for all the edits, I was slowly getting my bearings and things just unfolded… I will try my best to not always do that :)

@benhalverson Can we work on t/eslint together to avoid needing the obvious coercion… I know it is an acquired taste but onClick's typings should allow the typechecker to throw if my function is doing something wrong, and coercion here basically dilutes from having this clarity.