Rmwc: <Select /> onChange handler does not trigger in jsdom

Created on 4 Apr 2020  路  12Comments  路  Source: jamesmfriedman/rmwc

  • What RMWC Version are you using [major.minor.patch]: 6.0.9

  • Name your build system [Webpack, Rollup...]: CRA 2.0

  • Describe the bug with as much detail as possible:

Select onChange has changed in 6.0 to a CustomEvent, where it was previously (in 5.7.2) a SyntheticEvent.

Things appear to work fine in browsers:
5.7.2: https://codesandbox.io/s/rmwc-572-select-events-kbvlz
6.0.9: https://codesandbox.io/s/rmwc-609-select-events-pbrw3 (note also that the inputRef= never returns a ref too)

however in jsDOM environments, any tests that rely on triggering 'change' fail.

  • What happened, and what was supposed to happen:

In a jest test, fireEvent.change(submitEl, ...) would trigger onChange in 5.7.2

In 6.0.x, it does not trigger onChange.

You can see a minimal repro here:

5.7.2 - tests pass: https://github.com/tstirrat/rmwc-select-change-event/tree/working-5.7.2
6.0.9 - test fails: https://github.com/tstirrat/rmwc-select-change-event (master branch)

Also note that in 6.0.9 the label is not enclosed in a <label>, so I had to do some weird getByText dom traversal.

MDC bug question

Most helpful comment

@astrofrans I'll be upgrading this library to the newest version of material-components-web in the near future. These maintenance upgrades are usually not that bad, except in the last 2 versions they have decided to continuously refactor the select. Not only is this maddening for me, it also causes more issues such as this. I can definitely look into the issue, but not before I have to re-rewrite the select again.

All 12 comments

I cannot find a way to sanely simulate a change event in jsdom tests btw.

I've tried Simlate.change.

I notice that in 6.0, CustomEvent type is onChange and not SyntheticEvent of change too.

I even tried fireEvent(element, new CustomEvent('onChange')) and this did nothing, too.

MDC rewrote the select (again) and changed the markup. Thank you for the repro鈥檚, I鈥檒l get these fixed and look into the onChange.

Great, for now I've kept select at 5.7.2 and the rest at 6.0.9 in our app

@tstirrat dug into this for a bit. It's caused by an issue that had to be worked around in Google's code. Effectively, material-components-web fires the onChange handler on init. This definitely is NOT what we want, since it is contrary to how standard selects behave in React. Because of this, I had to employ a workaround to prevent onChange events from happening immediately on mount.

For the time being, here is a workaround which just involves delaying your onChange event firing.

test('renders learn react link', (done) => {
  const onChange = jest.fn();
  const { container } = render(<Select onChange={onChange} />);

  const select = container.children[0].querySelector('select');

  act(() => {
    window.requestAnimationFrame(() => {
      fireEvent.change(select!);
      expect(onChange).toHaveBeenCalled();
      done();
    });
  });
});

Let me know if that works for you. Ideally I could fix this and make it behave exactly like a native select, but due to the complicated nature of the integration with material-components-web under the hood I don't know if it would be possible.

Thanks, I'll try that!

This workaround did work, thanks!

Unfortunately, it's not completely working. I am using an async rAF delay to fire the event. this works, it fires the onChange. but in testing, it doesn't include the select's values:

    expect(jest.fn()).toHaveBeenCalledWith(...expected)

    Expected: "false"
    Received: "true"

    Number of calls: 1

      33 |
      34 |   expect(onChange).toHaveBeenCalledTimes(1);
    > 35 |   expect(onChange).toHaveBeenCalledWith('false');
         |                    ^
      36 | });
      37 |

      at Object.<anonymous> (src/App.test.js:35:20)

  console.log src/App.test.js:10
    onChange CustomEvent { isTrusted: [Getter] }.  <-------------- no (target|currentTarget).value

Test Suites: 1 failed, 1 total

Code updated here using the latest 6.0.14: https://github.com/tstirrat/rmwc-select-change-event

Using async firing:

  await act(() => {
    return new Promise((resolve) => {
      requestAnimationFrame(() => {
        fireEvent.change(select, {
          target: { value: 'false' },
        });
        resolve();
      });
    });
  });

  expect(onChange).toHaveBeenCalledTimes(1);
  expect(onChange).toHaveBeenCalledWith('false');

Well, I hate that you're having these issues. Did a little more investigating

  • Internally ,RMWC uses the "selectedIndex" property of the DOM event
  • I assumed that passing in the appropriate like so would have fixed the issue
await act(() => {
    return new Promise((resolve) => {
      requestAnimationFrame(() => {
        fireEvent.change(select, {
          currentTarget: { selectedIndex: 1 },
        });
        resolve();
      });
    });
  });

That doesn't appear to get us any closer, but it seems like anyone would have this exact same issue if they're using the select change in a unit test, so I'd really like to figure it out.

Thanks for investigating more. Yes I think it will surface with others too.

Perhaps a scope creep on this ticket, but I'm having issues with the onChange for the Select field too since upgrading to v.6.x.y , but in MSIE11.

The target and customTarget props are not copied correctly into the CustomEvent in the emitFactory (here) when run in MSIE11.

This is really unfortunate as I need to have support for internet explorer 11, and this makes select fields useless in that environment.

I am polyfilling custom events, so I'm pretty confident that something is wrong with the referenced line.

Would really appreciate some help on this! 馃檹

@astrofrans I'll be upgrading this library to the newest version of material-components-web in the near future. These maintenance upgrades are usually not that bad, except in the last 2 versions they have decided to continuously refactor the select. Not only is this maddening for me, it also causes more issues such as this. I can definitely look into the issue, but not before I have to re-rewrite the select again.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

robcaldecottvelo picture robcaldecottvelo  路  6Comments

istisiki picture istisiki  路  4Comments

jamesmfriedman picture jamesmfriedman  路  6Comments

UFOMelkor picture UFOMelkor  路  6Comments

hronro picture hronro  路  3Comments