find and findAll helpers do not use HTML element context properly

[jbingham94]

It seems that both the find and findAll helpers do not properly use context if you pass in an HTML element. For example:

find('.my-selector', myHTMLElement); // I want to look for .my-selector within myHTMLElement

Instead of looking within myHTMLElement, the helpers seem to look within the first element with myHTMLElement's selector. When I switch back to using find from ember-native-dom-helpers, the above code works properly.

[Turbo87]

the reason for this is that the find() functions in this library here don't have a context parameter. see https://github.com/emberjs/ember-test-helpers/blob/master/API.md#find

[rwjblue]

We should probably add an assertion to help make this clearer (and prevent folks from having to track it down)...

[jbingham94]

@Turbo87 Thanks for clearing that up. I don't feel like this would be super tough to implement, and it would definitely be a very helpful feature, as almost every find/findAll we do has some sort of context. This limitation is pretty much the only thing stopping us from ditching ember-native-dom-helpers completely.

[synaptiko]

Are you willing to reconsider adding context? It would match quite well with assert.dom() from qunit-dom.

[Turbo87]

qunit-dom is doing a bit more and have a slightly different API though.

instead of find('.my-selector', myHTMLElement); you can just use myHTMLElement.querySelector('.my-selector');

[rwjblue]

To reiterate from prior conversations on this, the find / findAll variants should generally only be used in tooling without access to the test context directly, user-land test code should totally use this.element directly.

The path forward here to prevent accidental trollage is to trigger an error when passing a second argument. As mentioned in #351 (comment).

[synaptiko]

@rwjblue We recently refactored our addons and apps from Ember 2.18 to 3.5. Usage of find/findAll vs this.element.querySelector/All was one of the things we were considering but it wasn't clear to us that find/findAll is not intended to be used by user-land test code. Especially when the default behavior of https://github.com/simonihmig/ember-test-helpers-codemod is usage of find/findAll. This was something which confused us. The fact there is an additional codemod to convert it to this.element didn't help.

[rwjblue]

The most likely reason for it to transform to find is just that it is easier to codemod (you don't have to worry about tracking which this context a given invocation has to know if you can use this.element or not), but lets move the convo about the codemod over into simonihmig/ember-test-helpers-codemod ...

[gustaff-weldon]

@rwjblue I find this confusing as well. If Ember team wants to push away from jQuery towards using native querySelector/querySelectorAll, why is there even an effort to create find/findAll helpers?
Shouldn't the native way also be the one to use in tests?

[rwjblue]

If Ember team wants to push away from jQuery towards using native querySelector/querySelectorAll, why is there even an effort to create find/findAll helpers?
Shouldn't the native way also be the one to use in tests?

Yes! That is exactly what I said in #351 (comment) above, in both of the RFCs (emberjs/rfcs#268 and emberjs/rfcs#232) and in #278.

[rwjblue]

#358 implemented assertions that I was referring to in #351 (comment). Closing...