fix: add functional HACK for triggering React change event

[shamrt]

Change

Add a functional hack when working with React-in-Ember apps or addons.

My team is using https://github.com/rewardops/ember-react-components (which is a modernization of https://github.com/alexlafroscia/ember-react-components) in order to bake React components into our Ember apps and addons. As you can see in the JSDoc in my changes, this hack is necessary for preparing React <input /> elements for change events. Without this preparation, <input />s ignore the simulated event following fillIn and typeIn actions.

Further background on this issue can be read by following the links in the JSDoc. (Note that this a problem for folks in the Selenium ecosystem as well.)

Rationale

We realize that this addition to the test-helpers library is rather niche. Normally, we would have forked this library, published our own version and gone on our merry way. However, much of the Ember testing ecosystem (e.g., https://github.com/emberjs/ember-qunit, https://github.com/san650/ember-cli-page-object) presupposes the use of @ember/test-helpers and has that dependency baked right in. As a result of this fairly tight coupling, leveraging our own forked (and renamed) library causes all sort of errors during testing, and so precludes us from going that direction — unless we also want to fork every other library that relies on this one.

As a temporary measure, we've added a tar.gz pack in our forked repo (see https://github.com/rewardops/ember-test-helpers/releases/tag/v2.6.0), but we hope this is not a long-term solution. Instead, we're hoping that the fix we propose here is negligible enough to warrant inclusion in the origin library. We're certainly open to feedback! 🙂

[rwjblue]

Hmm. This seems kinda odd to embed into this library for all Ember apps out of the box. 🤔

[rwjblue]

Instead of this, maybe you could leverage the hooks system?

ember-test-helpers/addon-test-support/@ember/test-helpers/dom/fill-in.ts

Lines 12 to 14 in 7d0aab9

	registerHook('fillIn', 'start', (target: Target, text: string) => {
	log('fillIn', target, text);
	});

See an example here:

https://github.com/ember-a11y/ember-a11y-testing/blob/e24c942830420ed18b67c8b23faef82c5d3536c7/addon-test-support/setup-global-a11y-hooks.ts#L73-L77

Note: this is still private API / hacks, but seems better to me than embedding things here...

[rwjblue]

I think you'd basically do something like:

import { _registerHook } from '@ember/test-helpers';

_registerHook('fillIn', 'start', (target: Target, text: string) => { 
  const tracker = target._valueTracker;
  if (tracker) {
    tracker.setValue('');
  }
});

[shamrt]

Great idea! Unfortunately, though, order matters when executing this hack, as you can see in facebook/react#11488 (comment). I attempted to implement this using both hooks (start and end) on the fillIn helper, but it didn't work. We need to (1) change the element value, (2) update the _trackerValue, then (3) fire the DOM event, in that order.

Perhaps if we added a new 'before-fire-event' hook in the helper — and if so then likely in other, similar helpers — then I could get it to work and throw a setup* helper into https://github.com/rewardops/ember-react-components.

Would that work? I'd be happy to submit a PR for the new hooks

[rwjblue]

Ya, I think having a runHooks('fireEvent', 'before-dispatch', element, event) and runHooks('fireEvent', 'after-dispatch', element, event) surrounding this line:

ember-test-helpers/addon-test-support/@ember/test-helpers/dom/fire-event.ts

Line 131 in 7d0aab9

element.dispatchEvent(event);

Would that work?

[rwjblue]

We could also add a start and end one too, at the beginning of fireEvents and at the end for good measure; but those wouldn't neccesarily work for your use case, right?

[shamrt]

Ah, sure, I could try something like that! I take it the PR I submitted is inadequate? If so, I can create a new one 🙂

[rwjblue]

I take it the PR I submitted is inadequate?

It was a great starting point for the discussion here!! But yes, I think we probably are very unlikely to move forward with react support code directly living here...

[shamrt]

Closing in favour of #1185