Material-ui: TypeScript error with v4.0.0-alpha.8

[nareshbhatia]

The following code worked perfectly fine in alpha.7, but is broken in alpha.8. Can't tell if it's React signatures that have changed or MUI's:

export interface ChartHeaderProps {
    onChange: (event: React.ChangeEvent<HTMLSelectElement>) => void;
}

export const ChartHeader = ({onTimePeriodChange}: ChartHeaderProps) => {
    return (
        <div className={classes.rhs}>
            <FormControl className={classes.formControl}>
                <Select
                    name="period"
                    value={timePeriod}
                    onChange={onChange}
                >
                    ...
                </Select>
            </FormControl>
        </div>
    )
};

Now I am getting the following TypeScript error:

Type '(event: ChangeEvent) => void' is not assignable to type '(event: ChangeEvent<{ name?: string | undefined; value: unknown; }>, child: ReactNode) => void'

I got past the error by changing the interface as follows, but that is a fairly complex signature for something so simple.

export interface ChartHeaderProps {
    onTimePeriodChange: (event: ChangeEvent<{ name?: string | undefined; value: unknown; }>, child: ReactNode) => void;
}

And it doesn't solve the problem. In the actual event handler, I am no longer able to use event.target.value, I get yet another TypeScript error:

Type 'unknown' is not assignable to type '(prevState: string) => string'

P.S.

There was one more TypeScript related error with makeStyles:

TS2571: Object is of type 'unknown`

I got around that one by adding the following import:

import { Theme } from '@material-ui/core/styles';

and then supplying the Theme type to makeStyles:

const useStyles = makeStyles((theme: Theme) => ({
    ...
}));

[eps1lon]

and then supplying the Theme type to makeStyles:

makeStyles from @material-ui/styles is a generic solution. The theme type has to be provided explicitly. From @material-ui/core it should default to a well defined Theme though.

Now I am getting the following TypeScript error:

Type '(event: ChangeEvent) => void' is not assignable to type '(event: ChangeEvent<{ name?: string | undefined; value: unknown; }>, child: ReactNode) => void'

Yes this was never correct. We pass our own target proxy that includes the actual value. See #15245 for more context. I guess the changelog entry wasn't really that descriptive.

[nareshbhatia]

@eps1lon, sorry but even after reading the links I am not clear on what the correct usage of the Select onChange is.

1. What is the signature of the onChange handler?
2. Where is the value stuffed in the event?

[eps1lon]

What is the signature of the onChange handler?

https://next.material-ui.com/api/select/#props

Is probably a bit vague. We can improve the documentation there.

https://unpkg.com/@material-ui/core@4.0.0-alpha.8/Select/SelectInput.d.ts

Where is the value stuffed in the event?

This shouldn't concern you for usage. If you want to learn more about the implementation (never rely on the implementation) you can check out:
https://unpkg.com/@material-ui/core@4.0.0-alpha.8/Select/SelectInput.js

Search for event.target =

[nareshbhatia]

@eps1lon, thanks for the pointers.

Based on SelectInput.d.ts, setting the event handler signature to (event: React.ChangeEvent) => void) (or similar variations) does not work (as indicated above). That type definition doesn't seem right.

Based on https://next.material-ui.com/api/select/#props, if I set the signature to (event: object) => void then I run into the following error on the usage side:

Property 'target' does not exist on type 'object'. TS2339

for this code:

const handleTimePeriodChange = (event: object) => {
    setTimePeriod(event.target.value);
};

So the only way I found to stop the compiler from complaining is to type event as any:

const handleTimePeriodChange = (event: any) => {
    setTimePeriod(event.target.value);
};

P.S. I was asking about where the value is stuffed, just so that I can get it out correctly. Now I know that the proper way is indeed event.target.value, but it seems tough to make the compiler understand that.

[nareshbhatia]

Curious, why is https://unpkg.com/@material-ui/core@4.0.0-alpha.8/Select/SelectInput.d.ts defines onChange as

onChange?: (
    event: React.ChangeEvent<{ name?: string; value: unknown }>,
    child: React.ReactNode,
  ) => void;

Why not just

  onChange?: (
    event: React.ChangeEvent<HTMLSelectElement>,
    child: React.ReactNode,
  ) => void;

If I patch SelectInput.d.ts with this definition, all problems go away. But I am sure there's a reason for why it is this way!

[eps1lon]

If I patch SelectInput.d.ts with this definition, all problems go away. But I am sure there's a reason for why it is this way!

Because the target is not an HTMLSelectElement but our own interface that preserves the original value. The current definition reflects the actual value. Only the value type is unknown because we can't determine the value at compile-time.

[nareshbhatia]

Makes sense.

To summarize, I am sticking with the following. That's the only way I can seem to make TypeScript happy:

const handleTimePeriodChange = (event: any) => {
    setTimePeriod(event.target.value);
};

[eps1lon]

Makes sense.

To summarize, I am sticking with the following. That's the only way I can seem to make TypeScript happy:

const handleTimePeriodChange = (event: any) => {
    setTimePeriod(event.target.value);
};

When in doubt first check out our demos. They are available in TypeScript. We try to be as strict as possible without adding additional runtime overhead. https://next.material-ui.com/demos/selects/#native-select

[nareshbhatia]

@eps1lon, that was super helpful! I am now able to type this a lot better. One type cast needed, but I fully understand why.

// ChartHeader.tsx
export interface ChartHeaderProps {
    onTimePeriodChange: (event: React.ChangeEvent<{ value: unknown }>) => void;
}

// AnalyzePage.tsx (consumes ChartHeader)
const handleTimePeriodChange = (event: React.ChangeEvent<{ value: unknown }>) => {
    setTimePeriod(event.target.value as string);
};

Here's the source for anyone who hits this issue in the future.

[TroySchmidt]

The makeStyles signature change in .8 is causing TypeScript linting issues for me as well. Had to override type the theme to Theme to make it happy.

[eps1lon]

The makeStyles signature change in .8 is causing TypeScript linting issues for me as well. Had to override type the theme to Theme to make it happy.

@TroySchmidt Please open a separate issue.