Material-ui: withWidth does not pass theme prop further

[esentsov-contorra]

When wrapping withWidth with withTheme, theme property does not appear. I checked the source and I believe this is because of that line
https://github.com/mui-org/material-ui/blob/f46b77827cbecad926eb113bad0fb6a695f94e40/src/utils/withWidth.js#L84

I suppose it should check somehow if theme prop should be passed further.

I'm using 1.0.0-beta.24 version.

Here is a small example of the issue (typescript)

function Test(props: WithTheme) {
    return (
        <div>
            {JSON.stringify(props)}  // no theme property here
        </div>
    );
}

let MobileDialog = withWidth()<WithTheme>(Test);
let WTheme = withTheme()(MobileDialog);

class App extends React.Component {
    render() {
        return (
            <div className="App" style={{padding: 16}}>
                <WTheme/>
            </div>
        );
    }
}

[oliviertassinari]

@esentsov-contorra Good catch, this is a tricky issue!

[oliviertassinari]

What do you think of not changing the default behavior but adding a withTheme option like with have for the withStyles higher-order component?

[esentsov-contorra]

This may be a solution, but for me it looks a bit strange to have such an option for the theme-unrelated components. Then it should also be added to withMobileDialog and other components (if any), that uses withWidth in their implementation.
There can be other clashes with frequently used names like 'width'. Might it be worth to add a general way to handle name clashes in HOC?
For example make all the HOC in the library to accept a name, so when one uses HOC in another HOC, it would be possible to rename the properties to avoid clashes.
Here is an example to show the idea

type WithTheme<K extends string = string> = {
    [P in K]: Theme;
    };

function Test(props: WithTheme<'theme'> & WithWidthProps) {
    return (
        <div>
            {JSON.stringify(props)}
        </div>
    );
}

function withTheme<P = {}, K extends string = 'theme'>(name: keyof WithTheme<K>) {
    return (Component: React.ComponentType<P & WithTheme<K>>) => {
        return class extends React.Component<P> {
            private testTheme = createMuiTheme();

            render() {
                let addedProps = {} as WithTheme<K>;
                addedProps[name] = this.testTheme;
                return (
                    <Component {...this.props} {...addedProps}/>
                );
            }
        };
    };
}

function withWidth<P = {}>(Component: React.ComponentType<P & WithWidthProps>) {
    const result = class extends React.Component<P & WithTheme<'test_theme'>> {
        render() {
            const {test_theme, ...others} = this.props as { test_theme: Theme };
            return (
                <Component {...others} width="xl"/>
            );
        }
    };
    return withTheme<P, 'test_theme'>('test_theme')(result);
}

let WWidth = withWidth<WithTheme<'theme'>>(Test);
let WTheme = withTheme('theme')(WWidth);

class App extends React.Component {
    render() {
        return (
            <div className="App" style={{padding: 16}}>
                <WTheme/>
            </div>
        );
    }
}

What do you think?

[oliviertassinari]

Might it be worth to add a general way to handle name clashes in HOC?

@esentsov-contorra This is one of the limitations with HOC. I don't think that the clash is the issue here. The issue is withWidth not forwarding the property. We could expose a compound API.

For example make all the HOC in the library to accept a name

This will make all the HOCs leaking an extra property, this goes against the design choice of making the withWidth not leaking the theme property. HOCs shouldn't spoil their children with unrelated properties.
I'm proposing a withTheme option to keep the component isolated. For me, the question is really about: Should be withTheme true or false by default?

[esentsov-contorra]

The proposed solution just helps to prevent extra properties leaking by renaming injected properties to smth component-specific, so it's easy to separate component-specific properties from others.
In this example withWidth component would call withTheme('withWidthTheme')(...) and gets injected property named withWidthTheme and does not pass it down to the wrapped component. If an outer component wraps withWidth with one more withTheme call, it passes another name for the injected property, so it does not clash with the withWidthTheme. Please check the code sample provided.

If you are still for the adding a withTheme option, I think the default should be false, just not to spoil children with the theme property again.

[oliviertassinari]

@esentsov-contorra we could also add a prefix option to the HOC factory so people can avoid clashes by prefixing the properties injected. But I'm not sure it worth the effort. Working on a compound API might be simpler.

[CharlieIGG]

Hey guys! Sorry to revive an old issue, but I'm bumping into something related and I just don't understand how to proceed.

I'm using withMobileDialog to wrap a dialog component with some custom props like so:

interface Props {
    fullScreen?: boolean,
    open: boolean,
    info?: string,
}

const InformationDialog = (props: Props) => {
    return (
        <Dialog
            fullScreen={props.fullScreen}
            open={props.open}
        >
           {props.info}
        </Dialog>
    );
}

export default withMobileDialog()(InformationDialog);

but when I try to render it on it's parent component like so:

<InformationDialog
    open={true}
    fullScreen={false}
    info={"Some Info Here"}
/>

I am getting this TS compilation error:
Type '{ open: boolean; fullScreen: boolean, info: string; }' has no properties in common with type 'IntrinsicAttributes & Partial<WithWidth> & { children?: ReactNode; }'.

How can I either fix or circumvent this @oliviertassinari @esentsov-contorra ?

[eps1lon]

You are missing width: Breakpoint in your props definition. Just look up what Partial<WithWidth> defines. The error message is actually helpful here.

[CharlieIGG]

Do you mean like this @eps1lon ?

interface Props {
    fullScreen?: boolean,
    open: boolean,
    info?: string,
    width: Breakpoint
}

const InformationDialog = (props: Props) => {
    return (
        <Dialog
            fullScreen={props.fullScreen}
            open={props.open}
        >
           {props.info}
        </Dialog>
    );
}

export default withMobileDialog()(InformationDialog);

This results in the same error, so I'm probably not understanding correctly.

I'm still not well acquainted with TS so that might be it, if this is a newbie problem, please just point me out what should I be learning.

[CharlieIGG]

I thinkI figured it out, I needed to pass props like so:

export default withMobileDialog<Props>()(InformationDialog);

[jflord78]

@CharlieIGG, thanks, this saved me a ton of aggravation :-)