spy on component method failed

[tpai]

I'm now using class properties feature on component method, but the spy seems not work.

The component like this:

// Sample.js
export default class Sample extends Component {
  sampleMethod = () => {
    console.log("sample");
  }
  render() {
    return (<button onClick={this.sampleMethod}>Sample</button>);
  }
}

Test code here:

// Sample.test.js
import { mount } from "enzyme";
import expect from "expect";
import Sample from "./Sample.js";

const wrapper = mount(<Sample />);
const spy = expect.spyOn(wrapper.instance(), "sampleMethod");
expect(spy).toNotHaveBeenCalled();
wrapper.find("#sample").simulate("click");
expect(spy).toHaveBeenCalled();

The result is spy was not called, is that class properties issue or just I wrote wrongly?

[ljharb]

The problem is that the component is rendered before you spy on it, and so the onClick is already bound to the original.

It is a class properties issue in that, if you made it a prototype method, you could spy on Sample.prototype.sampleMethod before calling mount, and it would work. As it is, I think you'd probably have to .update() the wrapper after spying, and before your assertions and simulate call.

[tpai]

Thanks for reply! :)

But I did what you said before this, there's no sampleMethod in Sample.prototype.

[ljharb]

@tpai right, because you're using class properties. I'm suggesting your component instead be:

// Sample.js
export default class Sample extends Component {
  constructor(...args) {
    super(...args);
    this.sampleMethod = this.sampleMethod.bind(this);
  }

  sampleMethod() {
    console.log("sample");
  }

  render() {
    return (<button onClick={this.sampleMethod}>Sample</button>);
  }
}

and then in your test:

// Sample.test.js
import { mount } from "enzyme";
import expect from "expect";
import Sample from "./Sample.js";

const spy = expect.spyOn(Sample.prototype, "sampleMethod");
const wrapper = mount(<Sample />);
expect(spy).toNotHaveBeenCalled();
wrapper.find("#sample").simulate("click");
expect(spy).toHaveBeenCalled();

[tpai]

@ljharb Got it! Thanks for the suggestion :)

So is there no any solution with class properties issue? Just lazy to bind method manually... :P

[ljharb]

wrapper.update() after the spyOn is the other alternative.

[tpai]

@ljharb IT WORKED! Thanks dude, you save my day. :)

[farzd]

are you using babel to transpile your code? I have the exact same code but get this error?
TypeError: (0 , _chai.mount) is not a function

[SerzN1]

@ljharb THANKS!

[samrae7]

Thank you for this @ljharb :) I was having a similar issue and noticed that the spy would be called only when I simulated a click twice. This was because I was spying on the method after mounting the component, so the spy was not attached to the mounted component. Once I had clicked once, that would be enough to 'update' the component, so then when I click again the spy was called. I fixed this by doing what you wrote. Now it works with one click as expected

[Wildhoney]

Remember to spy.restore() after your test has completed, otherwise the spy will persist across tests – you also won't be able to spy again on the same prototype method unless it was restored.

[swenyang]

I using component method as class properties, however, calling wrapper.update() after spyOn doesn't work for me. Calling another wrapper.instance().forceUpdate() solved my problem.

I'm using jest 22.1.3.

export default class Button extends React.Component {
    // ...
    handleTouchTap = () => {
        // ...
    }
    // ...
    render() {
        return <button onTouchTap={this.handleTouchTap}>{this.props.children}</button>
    }
}

const callback = jest.fn()
const wrapper = shallow(
    <Button onTouchTap={callback}>Sample</Button>
)
const handleTouchTap = jest.spyOn(wrapper.instance(), 'handleTouchTap')
wrapper.instance().forceUpdate()
wrapper.update()
wrapper.find('button').first().simulate('touchTap')
expect(callback.mock.calls.length).toBe(1)
expect(handleTouchTap.mock.calls.length).toBe(1)

[ljharb]

Spying on instance methods won’t work properly if it’s using arrow functions in class properties - avoid them; use constructor-bound instance methods instead.

[siemiatj]

But we're given the arrow functions to avoid binding manually. So this is going back from ES7 to ES5 :(

[ljharb]

@siemiatj First of all, class fields isn't in ES7; it's still stage 3, so it's ES nothing. Second, ES5 remains the only correct way to do this, I'm afraid. Use things because they're correct, not because they're shiny and new :-)

[siemiatj]

Well yeah I know. I think we're just getting lazy because of this new syntax sugar.

[GarrettGeorge]

So is there no way to mock/spy on these arrow functions in class properties just to have a toHaveBeenCalled test pass?

[ljharb]

@GarrettGeorge no; refactor them to be correct (ie, not arrow functions in class properties) and everything will be fine.

[GarrettGeorge]

@ljharb yeah I've just embraced it. However, I'm completely turned around and can't even figure out how to do the testing normally.

describe('SuggestedTags', () => {
	let wrapper;

	beforeEach(() => {
		wrapper = shallow(<SuggestedTags />)
	})

	it('should render nothing in the default state when state.suggestedTags is empty', () => {
		expect(wrapper.getElements()).toMatchSnapshot()
	});

	it('should render Mozart correctly when state.suggestedTags is [`Mozart`]', () => {
		wrapper.setState({ suggestedTags: ['Mozart'] })
		expect(wrapper.getElements()).toMatchSnapshot()
	})

	describe('handleClick', () => {
		let component;

		beforeEach(() => {
			component = wrapper.instance()
			wrapper.setState({ suggestedTags: ['Mozart'] })
		})

		it('should call handleClick when a tag is clicked', () => {
			wrapper.handleClick = jest.fn()
			wrapper.find('.suggested-tags__item').simulate('click')
			expect(component.handleClick).toHaveBeenCalled()
		})
	})
});

Do I ditch wrapper.instance() in favor of the wrapper itself?

When do I spyOn/jest.fn()?

[ljharb]

You have to do the spying before creating the wrapper. Specifically, I’d suggest creating the wrapper Isidre each it rather than using a beforeEach (DRY is often counterproductive in tests)

[dawnmist]

Spying on instance methods won’t work properly if it’s using arrow functions

This is a frustrating step backwards, because it used to work with React 15.6 & enzyme 2.9.1. I had put a personal project aside for a while, and this weekend decided to update it to newer dependencies...only to find that when it comes to testing what used to work now not only doesn't work but has no upgrade path possible except to change the code to fit the testing tools (yuck!).

Adding *.bind(this) in the constructors of every class simply adds more boilerplate and runs the risk for forgetting to bind newly added functions all the time. I hate having to remember to do that - and it's not typically an oversight that editors help you to find.

I can understand that the big changes with React 16 would have altered how enzyme does things, but I really do think that being incapable of mocking/spying on instance methods is an issue that needs to be addressed.

[ljharb]

I wouldn't expect it to have worked in React 15 and/or in enzyme 2; this isn't a new issue, it's an inherent property of using arrow functions in class properties, and it's why they never have, and never will, be a good idea.

Can you show me the code that passed in enzyme 2 and react 15?

[dawnmist]

Working with Typescript, here is a single component and the tests that were run against it. I've tried to cut down the dependencies as much as possible so that it could be examined in isolation.

With the given package versions, jest.spyOn was able to spy on the onChange, onSyncClick, handleReset and handleSubmit functions.

Critical package versions:

{
    "@types/enzyme": "^2.8.9",
    "@types/enzyme-to-json": "^1.5.0",
    "@types/jest": "^20.0.8",
    "@types/react": "^15.6.4",
    "@types/react-bootstrap": "^0.0.52",
    "@types/react-dom": "^15.5.5",
    "@types/react-test-renderer": "^15.5.4",
    "bootstrap": "^3.3.7",
    "enzyme": "^2.9.1",
    "enzyme-to-json": "^1.6.0",
    "jest": "^20.0.4",
    "jest-cli": "^20.0.4",
    "react": "^15.6.2",
    "react-addons-test-utils": "^15.6.2",
    "react-bootstrap": "^0.31.3",
    "react-dom": "^15.6.2",
    "react-scripts-ts": "^2.7.0",
    "react-test-renderer": "^15.6.2",
    "ts-jest": "^20.0.14",
    "tslint-react": "^3.2.0",
    "typescript": "^2.5.3"
  }

Component file DatabaseSettingsForm.tsx

import * as React from 'react';
import { Button, ButtonGroup, Col, ControlLabel, Form, FormControl, FormControlProps, FormGroup } from 'react-bootstrap';

export const DATABASE_SETTINGS = '_local/couchdb-sync';
export type DATABASE_SETTINGS = typeof DATABASE_SETTINGS;

export interface DbMeta {
  _id: string;
  _rev?: string;
  _deleted?: boolean;
}

export interface DatabaseSettings extends DbMeta {
  type: DATABASE_SETTINGS;
  url: string;
  username: string;
  password?: string;
  shouldSync: boolean;
}

export type FormControlEvent = React.FormEvent<FormControlProps>;

export interface DatabaseProps {
  databaseSettings: DatabaseSettings;
  updateSettings: (settings: DatabaseSettings) => void;
}

class DatabaseSettingsForm extends React.Component<DatabaseProps, DatabaseSettings> {
  state: DatabaseSettings;
  props: DatabaseProps;

  constructor(props: DatabaseProps) {
    super(props);
    this.state = this.props.databaseSettings;
  }

  componentWillReceiveProps(nextProps: DatabaseProps) {
    if (this.props.databaseSettings !== nextProps.databaseSettings) {
      const { databaseSettings } = nextProps;
      this.setState(databaseSettings);
    }
  }

  handleReset = (e: React.FormEvent<Form>) => {
    e.preventDefault();
    this.setState(this.props.databaseSettings);
  };

  onChange = (e: FormControlEvent) => {
    const { id, type } = e.currentTarget;
    const value = type === 'checkbox' ? e.currentTarget.checked : e.currentTarget.value;
    this.setState({ ...this.state, [id!]: value });
  };

  onSyncClick = () => {
    const { shouldSync } = this.state;
    this.setState({ ...this.state, shouldSync: !shouldSync });
  };

  handleSubmit = (e: React.FormEvent<Form>) => {
    e.preventDefault();
    this.props.updateSettings({ ...this.state });
  };

  render(): React.ReactElement<JSX.Element> {
    const { url, username, shouldSync } = this.state;
    const { databaseSettings } = this.props;
    const labelWidth = 3;
    const fieldWidth = 9;
    return (
      <Form horizontal={true} onSubmit={this.handleSubmit} onReset={this.handleReset}>
        <FormGroup controlId="url">
          <Col componentClass={ControlLabel} sm={labelWidth}>
            Master Database URL:
          </Col>
          <Col sm={fieldWidth} className="no-left-padding">
            <FormControl onChange={this.onChange} type="text" placeholder={databaseSettings.url} value={url} />
          </Col>
        </FormGroup>
        <FormGroup controlId="username">
          <Col componentClass={ControlLabel} sm={labelWidth}>
            Database Username:
          </Col>
          <Col sm={fieldWidth} className="no-left-padding">
            <FormControl
              onChange={this.onChange}
              type="text"
              placeholder={databaseSettings.username}
              value={username}
              name="username"
            />
          </Col>
        </FormGroup>
        <FormGroup>
          <Col smOffset={labelWidth} sm={fieldWidth} className="no-left-padding">
            <input type="checkbox" id="shouldSync" onClick={this.onSyncClick} checked={shouldSync} />
            <label htmlFor="shouldSync">Sync Databases</label>
          </Col>
        </FormGroup>
        <FormGroup>
          <Col sm={12}>
            <ButtonGroup justified={true}>
              <ButtonGroup>
                <Button type="submit">Apply Settings</Button>
              </ButtonGroup>
              <ButtonGroup>
                <Button type="reset">Cancel</Button>
              </ButtonGroup>
            </ButtonGroup>
          </Col>
        </FormGroup>
      </Form>
    );
  }
}

export default DatabaseSettingsForm;

Component Test File:

import { mount, shallow } from 'enzyme';
import toJson from 'enzyme-to-json';
import * as React from 'react';
import DatabaseSettingsForm, { DATABASE_SETTINGS, DatabaseSettings } from './DatabaseSettingsForm';

describe('Database Settings Form Component Tests', () => {
  const sampleSettings: DatabaseSettings = {
    _id: DATABASE_SETTINGS,
    type: DATABASE_SETTINGS,
    url: 'https://localhost:5834',
    username: 'myself',
    shouldSync: true
  };

  it('Should render the form', () => {
    const rendered = shallow(<DatabaseSettingsForm databaseSettings={sampleSettings} updateSettings={jest.fn()} />);
    expect(toJson(rendered)).toMatchSnapshot();
  });

  it('Should update the username field value when the user enters a value', () => {
    const rendered = mount(<DatabaseSettingsForm databaseSettings={sampleSettings} updateSettings={jest.fn()} />);
    const spyOnChange = jest.spyOn(rendered.instance() as DatabaseSettingsForm, 'onChange');
    const spySetState = jest.spyOn(rendered.instance(), 'setState');
    rendered.update();

    const input = rendered.find('[id="username"]');
    expect(input).toBeDefined();

    (input.getDOMNode() as HTMLInputElement).value = 'yourname';
    input.simulate('change');

    expect(spyOnChange).toHaveBeenCalledTimes(1);
    expect(spySetState).toHaveBeenCalledTimes(1);
    expect(spySetState).toHaveBeenCalledWith({ ...sampleSettings, username: 'yourname' });
    spyOnChange.mockReset();
    spySetState.mockReset();
    spyOnChange.mockRestore();
    spySetState.mockRestore();
  });

  it('Should update the url field value when the user enters a value', () => {
    const rendered = mount(<DatabaseSettingsForm databaseSettings={sampleSettings} updateSettings={jest.fn()} />);
    const spyOnChange = jest.spyOn(rendered.instance() as DatabaseSettingsForm, 'onChange');
    const spySetState = jest.spyOn(rendered.instance(), 'setState');
    rendered.update();

    const input = rendered.find('[id="url"]');
    expect(input).toBeDefined();

    (input.getDOMNode() as HTMLInputElement).value = '';
    input.simulate('change');

    expect(spyOnChange).toHaveBeenCalledTimes(1);
    expect(spySetState).toHaveBeenCalledTimes(1);
    expect(spySetState).toHaveBeenCalledWith({ ...sampleSettings, url: '' });
    spyOnChange.mockReset();
    spySetState.mockReset();
    spyOnChange.mockRestore();
    spySetState.mockRestore();
  });

  it('Should update the shouldSync field value when the user enters a value', () => {
    const rendered = mount(<DatabaseSettingsForm databaseSettings={sampleSettings} updateSettings={jest.fn()} />);
    const spyOnSyncClick = jest.spyOn(rendered.instance() as DatabaseSettingsForm, 'onSyncClick');
    const spySetState = jest.spyOn(rendered.instance(), 'setState');
    rendered.update();

    const input = rendered.find('[id="shouldSync"]');
    expect(input).toBeDefined();

    input.simulate('click');

    expect(spyOnSyncClick).toHaveBeenCalledTimes(1);
    expect(spySetState).toHaveBeenCalledTimes(1);
    expect(spySetState).toHaveBeenCalledWith({ ...sampleSettings, shouldSync: !sampleSettings.shouldSync });
    spyOnSyncClick.mockReset();
    spySetState.mockReset();
    spyOnSyncClick.mockRestore();
    spySetState.mockRestore();
  });

  it('Should submit the form with updated values', () => {
    const saveDbSettings = jest.fn();
    const rendered = mount(<DatabaseSettingsForm databaseSettings={sampleSettings} updateSettings={saveDbSettings} />);
    const spySubmit = jest.spyOn(rendered.instance() as DatabaseSettingsForm, 'handleSubmit');
    rendered.update();

    const username = rendered.find('input[id="username"]');
    expect(username).toBeDefined();
    (username.getDOMNode() as HTMLInputElement).value = 'yourname';
    username.simulate('change');

    const url = rendered.find('[id="url"]');
    expect(url).toBeDefined();
    (url.getDOMNode() as HTMLInputElement).value = '';
    url.simulate('change');

    const shouldSync = rendered.find('[id="shouldSync"]');
    expect(shouldSync).toBeDefined();
    shouldSync.simulate('click');

    const form = rendered.find('form');
    expect(form.html()).toBeDefined();

    form.simulate('submit');

    expect(spySubmit).toHaveBeenCalledTimes(1);
    expect(saveDbSettings).toHaveBeenCalledTimes(1);
    expect(saveDbSettings).toHaveBeenCalledWith({
      ...sampleSettings,
      username: 'yourname',
      url: '',
      shouldSync: !sampleSettings.shouldSync
    });
    spySubmit.mockReset();
    saveDbSettings.mockReset();
    spySubmit.mockRestore();
  });

  it('Should reset the form values to the original props when the form is reset', () => {
    const rendered = mount(<DatabaseSettingsForm databaseSettings={sampleSettings} updateSettings={jest.fn()} />);
    const spyReset = jest.spyOn(rendered.instance() as DatabaseSettingsForm, 'handleReset');
    rendered.update();

    const username = rendered.find('input[id="username"]');
    expect(username).toBeDefined();
    (username.getDOMNode() as HTMLInputElement).value = 'yourname';
    username.simulate('change');

    const url = rendered.find('[id="url"]');
    expect(url).toBeDefined();
    (url.getDOMNode() as HTMLInputElement).value = '';
    url.simulate('change');

    const shouldSync = rendered.find('[id="shouldSync"]');
    expect(shouldSync).toBeDefined();
    shouldSync.simulate('click');

    expect(rendered.state()).toMatchObject({
      ...sampleSettings,
      username: 'yourname',
      url: '',
      shouldSync: !sampleSettings.shouldSync
    });

    const form = rendered.find('form');
    expect(form.html()).toBeDefined();

    form.simulate('reset');
    expect(spyReset).toHaveBeenCalledTimes(1);
    expect(rendered.state()).toMatchObject(sampleSettings);
    spyReset.mockReset();
    spyReset.mockRestore();
  });

  it('Updates the display to match new props', () => {
    const spyReceiveProps = jest.spyOn(DatabaseSettingsForm.prototype, 'componentWillReceiveProps');
    const rendered = mount(<DatabaseSettingsForm databaseSettings={sampleSettings} updateSettings={jest.fn()} />);

    rendered.setProps({
      databaseSettings: {
        ...sampleSettings,
        username: 'yourname',
        url: '',
        shouldSync: !sampleSettings.shouldSync
      },
      saveDatabaseSettings: jest.fn()
    });

    expect(spyReceiveProps).toHaveBeenCalledTimes(1);
    expect(rendered.state()).toMatchObject({
      ...sampleSettings,
      username: 'yourname',
      url: '',
      shouldSync: !sampleSettings.shouldSync
    });
    spyReceiveProps.mockReset();
    spyReceiveProps.mockRestore();
  });

  it('Leaves the display unchanged if the props for the database settings are unchanged', () => {
    const spyReceiveProps = jest.spyOn(DatabaseSettingsForm.prototype, 'componentWillReceiveProps');
    const rendered = mount(<DatabaseSettingsForm databaseSettings={sampleSettings} updateSettings={jest.fn()} />);

    const url = rendered.find('[id="url"]');
    expect(url).toBeDefined();
    (url.getDOMNode() as HTMLInputElement).value = '';
    url.simulate('change');

    const shouldSync = rendered.find('[id="shouldSync"]');
    expect(shouldSync).toBeDefined();
    shouldSync.simulate('click');

    rendered.setProps({
      databaseSettings: sampleSettings,
      saveDatabaseSettings: jest.fn()
    });

    expect(spyReceiveProps).toHaveBeenCalledTimes(1);
    expect(rendered.state()).toMatchObject({
      ...sampleSettings,
      url: '',
      shouldSync: !sampleSettings.shouldSync
    });
    spyReceiveProps.mockReset();
    spyReceiveProps.mockRestore();
  });
});

[ljharb]

It seems like after you spy on the arrow function class property method, you call .update() - in other words, you were relying on some of enzyme v2's unintentional mutability. In production, this would be the same - if you spied on the method after creating the element, you'd have to force React to rerender - otherwise you wouldn't see the effect of your spy.

Again, this is just the nature of arrow functions in class properties. There's nothing enzyme can do to make it possible to spy on them without reverting much of the react decoupling and reliability improvements done for v3.

Use constructor-bound or class-field-bound (foo = this.foo.bind(this)) instance methods, spy on the prototype before creating the wrapper, and everything will work as you expect.

[brettcrowell]

Just came across this same class properties and spyOn situation myself, and noticed that tests do actually work when the component wraps the call in an arrow function...

// <SampleComponent/>
classPropertyFunction = () => false;
...
<a onClick={() => this.classPropertyFunction()}>

// test
const sampleComponent = shallow(<SampleComponent/>);
const classPropertyFunctionSpy = jest.spyOn(
  sampleComponent.instance(),
  "classPropertyFunction"
);

When called directly, though, the spy doesn't pick anything up...

// <SampleComponent/>
classPropertyFunction = () => false;
...
<a onClick={this.classPropertyFunction}>

Definitely not good to be creating arbitrary arrow function wrappers in render to get the tests to pass, but it does seem to be related.

[ljharb]

@brettcrowell yes, that's exactly (part of) why you want to constructor-bind instead of using arrows in class functions.

[audiolion]

I think its safe to say that many people prefer these ES nothing class fields, and inline declarations for simple functions, because they eliminate cognitive overhead. If it's something as trivial as onClick={() => this.setState({ modalOpen: true })}, creating a prototype method on the class and binding it on the constructor seems like overkill, and though it is more performant, it's a micro-optimization that yields no visible impact to the dev/user in my experience.

That being said, I understand Jordan's argument and I feel like there needs to be a push from the react community to better teach this distinction, perhaps a discussion among leading devs summarising the pros/cons and recommendations would be a good step towards educating the developer community.

[andybarron]

Will arrow function class properties be supported when they make it into the spec? Or will we still insist on not supporting a popular paradigm in Enzyme?

[ljharb]

@andybarron it's not a question of them being "supported"; the way they work in the language means that never, until the end of time, should you use an arrow function in a class field.

In other words, this isn't enzyme refusing to support a popular paradigm, this is a popular paradigm being incorrect in the context of javascript itself, and enzyme being powerless to overcome this.

[andybarron]

Ah, re-reading the thread, I think I see what you're saying. The semantics of class field arrow functions (not existing until the instance is created) means you can't pre-emptively spy on them. My bad!

[WilliamHolmes]

Wouldn't using an inline onClick={() => this.func())} result in a new function reference on every render?
In react this would essentially be a new prop passed on every parent render causing more renders calls on the child.

[ljharb]

@WilliamHolmes yes, this is true if the child is a custom component (and thus might be a pure component). That’s why you constructor-bind an instance method instead, so you get the same reference every time.

[DZuz14]

Peeps, its easy as this,

let cmp = shallow(<Slider />);
const renderSlides = jest.spyOn(cmp.instance(), 'renderSlides');
cmp.instance().forceUpdate();
expect(renderSlides.mock.calls.length).toBe(1);

I refuse to write bind this like an absolute heathen everytime. Keep pushing forward people!

[ljharb]

@DZuz14 requiring the forceUpdate is far messier than writing your component code properly solely because you presumably like the shininess of an arrow function in a class field.

Separately, don't use renderFoo methods - those should be separate SFCs.

[buddyp450]

@DZuz14 this also won't work if you're testing things that only do something when mounting

[DZuz14]

Why do you guys have to rain on my arrow function parade? I am going to start using bind.this now, because if you cant beat them, join them!

[ghost]

Hello,
Maybe my answer is in the conversation, but I didn't see it ...
When you test a stateless components and there's no instance (React 16), how are you doing ?

const spy = jest.spyOn(MyComponent, 'handleSelect')

I tried with MyComponent and MyComponent.prototype but first one returns TypeError: Cannot read property '_isMockFunction' of undefined, and second Cannot read property handleSelect of null

handleSelect is a prop of MyComponent, and I need to test this function is triggered when I click on a button.

[ljharb]

@psa-gbellencontre stateless components have nothing to spy on - you spy on instance properties, not on props. If you want to test the handleSelect prop, pass a spy to it.

[DZuz14]

I wish some prominent members of the React team could publicly address this issue. From what I see, the use of fat arrows in class properties and the use of inline arrow functions is becoming pretty widespread. I know a few companies that say they use them, but since I do not work for them, I am not really sure how they are testing their components, or if they are even testing them at all(you would be surprised the companies that don't do testing at all). Even people in the React community that could be seen as "trustworthy" provide code examples with fat arrows being used in this manner, which is obviously misleading people.

I am currently writing a style guide for my team, and am still wondering where arrow functions can be used, or if they should be used at all. For now, I guess I am going to say don't use arrow functions in the following ways:

• As class properties (Methods)
• Using in event handlers. So no: onClick={() => this.handleClick()}

I would really like to have a uniform way of testing that everyone can agree on. Using forceUpdate or instance() seems like a hack, which could potentially lead to problems down the road.

What is also confusing is that even the React docs show examples of arrow functions being used
https://reactjs.org/docs/handling-events.html#passing-arguments-to-event-handlers

@ljharb This thread has me second guessing if they should be used at all. Do you guys use them anywhere in your React code at Airbnb?

[ljharb]

@DZuz14 we don't use class fields at all at airbnb, but when we do, we will expressly forbid arrow functions in them, full stop.

[DZuz14]

I see. So obviously some people are against anonymous inline functions, as they get treated as a new instance on each re-render, but utilizing them should be fine for testing as long as the method it calls is bound inside the constructor? Very very simplistic example:

class Example extends Component {
   constructor(props) {
      super(props);     
      this.handleClick = this.handleClick.bind(this);
   }

   handleClick() {
       // do something great
   }

  render() {
     return (
         <div>
             <button onClick={() => this.handleClick}>
                 Click Me 
            </button>
         </div>
     )
  }
}

Just want to make sure I understand all of the implications of that.

[ljharb]

That's a separate issue - passing a function created in the render path as a prop to a custom component is a problem, because it might (now or later) be Pure. Passing it to an HTML element is totally fine (altho in your example, you either want onClick={this.handleClick} or onClick={() => this.handleClick()} without the bind; what you have now is incorrect).

[DZuz14]

Whoops. Yeah I meant to have the invocation of it at the end. Obviously this is a little off topic, but I am not exactly sure what you mean by "it might (now or later) be Pure". I do understand the concept of pure functions, but I would be lying if I said I understood exactly what you meant.

When you say "Pure", are you just referring to a functional React component? Thanks for answering these questions by the way.

[ljharb]

No, I mean extends React.PureComponent - a component that avoids unnecessary rerenders by only rendering when all the props are not === to those on the previous render.

[audiolion]

Jordan, just want to say, huge props for continually staying subscribed to this issue and re-answering the same exact question. Should we be pointing people to eslint-airbnb-config which will give them warnings when they are using these anti-patterns?

@DZuz14 if you pass an inline function in your render, it will be a different function every time, and so if you are using PureComponent or a shouldComponentUpdate lifecycle in the component receiving that function, you have to explicitly ignore it in the comparison, otherwise the child component will re-render every time as the previous inline function !== the new inline function. I think in general, React is fast enough and processors are fast enough where people don't see perf issues and just roll with it, but it can definitely lead to harder to maintain code and issues down the road.

[DZuz14]

Yeah I agree. The thread really is just the same thing over and over, but its been quite helpful to chat about this stuff, and might help other people who stumble upon it later. I've bookmarked the eslint config, and will check it out.

[shaodahong]

Spying on instance methods won’t work properly if it’s using arrow functions in class properties - avoid them; use constructor-bound instance methods instead.

@ljharb Now arrow function is standard, enzyme can support it?

[ljharb]

@shaodahong it's still stage 3, but no, it's got nothing to do with that - it is impossible because of the way the language works to ever support it, ever. Don't put functions in class fields.

[Geczy]

Peeps, its easy as this,

let cmp = shallow(<Slider />);
const renderSlides = jest.spyOn(cmp.instance(), 'renderSlides');
cmp.instance().forceUpdate();
expect(renderSlides.mock.calls.length).toBe(1);

I refuse to write bind this like an absolute heathen everytime. Keep pushing forward people!

2020 and this is what I had to do for it to work as well. Thank you for the solution

[ljharb]

Again, the solution is to not put functions in class fields, in 2019, 2020, or 2050. Write your code correctly and you won't have to use hacks to test it properly :-)

[Geczy]

Again, the solution is to not put functions in class fields, in 2019, 2020, or 2050. Write your code correctly and you won't have to use hacks to test it properly :-)

Is reactjs.org giving bad advice, then? https://reactjs.org/docs/handling-events.html

Why do you think this transformer exists? https://babeljs.io/docs/en/babel-plugin-transform-class-properties/

[ljharb]

@Geczy yes, it absolutely is, and has always done so with its use of arrow functions in class fields.

The babel transform exists because the spec allows for functions in fields (because it'd be weird for it not to permit it) and the transform has to follow the spec. That's utterly irrelevant to whether you should use certain patterns.