9

I'm referring to this github project and I'm trying to write a simple test of KeyPad.js component.

I have seen the issues opened on this matter and one suggested solution is to pass the theme as prop to the component. This solution wouldn't work with enzyme.

The problem in my case is that children components receives the theme through ThemeProvider and to be able to make the test works I would need to add theme prop to all.

Example:

const tree = renderer.create(
        <KeyPad 
            theme={theme}
            cancel={()=> true}
            confirm={()=> true}             
            validation={()=> true}
            keyValid={()=>true} />
      ).toJSON();
      expect(tree).toMatchSnapshot();

KeyPad's render method would change like this, with theme prop everywhere

render() {
        let { displayRule, validation, label, confirm, cancel, theme, keyValid, dateFormat } = this.props
        return (
            <Container theme={theme}>
                <Content theme={theme}>
                    <Header theme={theme}>
                        <CancelButton theme={theme} onClick={cancel}>
                            <MdCancel />
                        </CancelButton>
                        <Label>{label}</Label>
                        <ConfirmButton theme={theme} onClick={() => confirm(this.state.input)} disabled={!validation(this.state.input)}>
                            <MdCheck />
                        </ConfirmButton>
                    </Header>
                    <Display
                        theme={theme}
                        value={this.state.input}
                        displayRule={displayRule}
                        dateFormat={dateFormat}
                        cancel={this.cancelLastInsert} />
                    <Keys>
                        {[7, 8, 9, 4, 5, 6, 1, 2, 3, '-', 0, '.'].map( key => (
                            <Button
                                theme={theme}
                                key={`button-${key}`}
                                theme={theme} 
                                click={(key) => this.handleClick(key) }
                                value={key}
                                disabled={!keyValid(this.state.input, key, dateFormat)} />
                        ))}
                    </Keys>
                </Content>
            </Container>
        )    
    }

I don't like this solution. Anybody can help me with this?

Thanks

doelleri
  • 19,232
  • 5
  • 61
  • 65
Pietro
  • 1,815
  • 2
  • 29
  • 63

2 Answers2

0

I would just destructure the callback's props argument and add a default value for theme. This way, any prop you might access from theme will not throw with Cannot read property ... of undefined and will return undefined. It will mess up your styles but I don't think you care too much about that in your unit tests.

...
color: ${({ theme = {} }) => theme.background};
...
Ionut Achim
  • 937
  • 5
  • 10
-1

This is perfectly reasonable as far as i could see. Every component in the library has a theme prop and thus you can set it like this.

Luckily it doesn't use context otherwise you would be further from home. why not use context: https://reactjs.org/docs/context.html

If you don't do it with props you will end up coupling component to another external library. eg: Redux or a mobx store. This means it is no longer a true component

even though it looks terrible. it is the way to go if you truly want to make a separate component.

Joel Harkes
  • 10,975
  • 3
  • 46
  • 65
  • Are you suggesting to use context in all components instead of ThemeProvider? I don't see why coupling with StyledComponent would make the component no longer true. Styles can be overridden. – Pietro Nov 29 '17 at 14:29
  • @Pietro no im suggesting to keep it the way you have it. it is the best solution if you want `` to be a true react component – Joel Harkes Nov 29 '17 at 14:31