rfcs
rfcs copied to clipboard
DOM Element descriptor interface for test helpers
For reference, here is a PR I opened in qunit-dom
. I've since realized that this would ideally apply to @ember/test-helpers
as well, and should therefore be a more generalized and carefully thought out interface.
We could also make use of this in Ember's Application
class rootElement
property.
Some questions that popped up for me, not sure, if in scope though:
What if a descriptor matches multiple elements, but the function it is passed to expects a single element?
How would a "live" descriptor work, that defers looking up the element(s) until asked to? By implementing element
/ elements
as getters?
In the case of selector strings, could it be useful to retain the selector string in the descriptor, e.g for debugging purposes or re-evaluation?
We could also make use of this in Ember's Application class rootElement property.
Good point, I hadn't thought of that!
What if a descriptor matches multiple elements, but the function it is passed to expects a single element?
Element descriptors have roughly the same mental model as selectors -- they don't inherently choose between describing one matched element or all matched elements and, like selectors today, it's up to the semantics of the API method to decide what to do with them -- pass a selector to querySelector()
or querySelectorAll()
and act on the result, or access the element
or elements
property of an element descriptor and act on the result. A great illustration of this is qunit-dom
's assert.dom()
that accepts a selector, but allows you to call .hasClass()
, in which case it acts on the first matched element, or .exists({ count: 3 })
, in which case it acts on all matched elements.
So descriptors just expose element
and elements
and consumers choose which to access based on the context.
How would a "live" descriptor work, that defers looking up the element(s) until asked to? By implementing element / elements as getters?
Yes, exactly. Like selectors, the primary use case I have in mind is "live" descriptors that can describe something that may or may not be rendered at the time the descriptor is constructed. But making the interface use properties instead of methods makes it very simple to implement static/ad-hoc descriptors as well, e.g. { element, elements: [element] }
.
In the case of selector strings, could it be useful to retain the selector string in the descriptor, e.g for debugging purposes or re-evaluation?
Yes, that's the intent behind the description
field in IElementDescriptor
and my assumption is that most of the time element descriptors will have a selector as their description because it's by far the most common ways to refer to DOM elements. But fractal-page-object
, for example, supports list indexing, so the descriptions could end up looking like ui li[1] .checkbox
. But I do expect that we'll end up with some kind of function makeSelectorDescriptor(selector: string)
that will use the selector as the description.
what's the status of this? like a tldr, what's left / remaining / etc?
@NullVoxPopuli I'm not super familiar with the RFC process, so not sure what needs to be done there. I have been working on an implementation of the RFC, an @ember/test-helpers branch adding support for descriptors, an equivalent qunit-dom branch, and a fractal-page-object branch making PageObject
s IDOMElementDescriptor
s.
I've run into a few issues that may or may not require/motivate adjustments to this RFC:
Element array vs. iterable
The RFC currently types DescriptorData#elements
as Element[]
, and I think it should change to Iterable<Element>
since it follows a querySelectorAll()
-like pattern (and seems likely to be a wrapper around some form of querySelectorAll()
much of the time).
The resolveDOMElements()
helper should still flatten to an array for nice ergonomics, but the descriptor interface itself should deal in iterators to require less data transformation by descriptor implementations, and leave open the option to use iterators in performance-sensitive scenarios, etc.
IDOMElementDescriptor type safety
I'm finding myself a little un-satisfied with IDOMElementDescriptor
being an empty interface. Since it's an empty interface, any object can be implicitly cast to it, which seriously limits the help the compiler/typechecker can provide. Here's a contrived example:
class Page extends PageObject {
date = selector('.date');
listItems = selector('li');
// some data parsed out of the list items
get data(): Record<string, string> {
let data: Record<string, string> = {};
for (let li of this.listItems) {
let [key, value] = li.element.textContent.split(' ');
data[key] = value;
}
return data;
}
}
let page = new Page();
// Good, fine
assert.dom(page.date).exists();
// Whoops, typo, but the compiler can't tell me that there's a problem because it can
// implicitly cast `Record<string, string>` to `IDOMElementDescriptor`, so, I only discover
// my typo at runtime even though this is exactly the kind of thing the compiler is meant
// to help with
assert.dom(page.data).exists();
I don't know if there's any typescript wizardry to fix this, but I don't think so because of how interfaces are meant to work. So I'm kind of inclined to improve these end-developer-facing ergonomics by putting a tiny bit more burden on implementors of IDOMElementDescriptor
. My only idea for how to do this would be to export a symbol from dom-element-descriptors
and include that symbol in IDOMElementDescriptor
:
export const IS_DESCRIPTOR = Symbol('is descriptor');
export interface IDOMElementDescriptor {
[IS_DESCRIPTOR]: any;
}
so descriptors would have to
import { IDOMElementDescriptor, IS_DESCRIPTOR } from 'dom-element-descriptors';
export default class DescriptorThingy implements IDOMElementDescriptor {
[IS_DESCRIPTOR] = true;
// or, to keep it in the prototype
// [IS_DESCRIPTOR]() {}
}
But I'm definitely open to ideas here.
Library/dependency structure
My naive attempt at hooking this all together is not working out. Currently I have dom-element-descriptors
as a dependency
of @ember/test-helpers
, qunit-dom
, and fractal-page-object
. Because dom-element-descriptors
stores the descriptor data in a module-scoped weak map, if the different versions drift such that an application doesn't end up with just a single hoisted copy of dom-element-descriptors
, then different libraries might be using different copies, e.g. fractal-page-object
might register its descriptors in one weak map, while qunit-dom
tries to look them up in another and fails to find them.
This seems like a nasty footgun waiting to happen. So I'm wondering if we can figure out some better dependency structure than "everyone that needs dom-element-descriptors
specifies your own dependency." I hate peer dependencies, but I wonder if this is the right time for them. Another also-not-super-great option would be to make the weak map a global rather than module scoping it. I'm not sure what the right path is here, and I'm also not 100% sure this needs to be specified in the RFC vs. being an "implementation detail," but I do think we need to figure something out.
Conclusion
I think my current front-runners are to:
- change
DescriptorData#elements
toIterable<Element>
- add the symbol to
IDOMElementDescriptor
- make the weak map a global, at least for the time being (note that this won't affect any of the APIs, just how various libraries need to consume
dom-element-descriptors
)
If I don't hear otherwise, in a few days I'll update the RFC to specify (1) and (2), and continue assuming that (3) doesn't need to be specified in this RFC.
@bendemboski are you still interested in getting this moved forward? If so, I'll do what I can to facilitate.
@wagenet I definitely am! I have an implementation of the RFC put together as well as (fairly old and in need of rebasing) forks of fractal-page-object
, @ember/test-helpers
, and qunit-dom
that provide descriptor functionality. I'd love to get this moving again!
@bendemboski anything Core can do to help?
@wagenet I just made the updates to the RFC that I described in https://github.com/emberjs/rfcs/pull/726#issuecomment-908909252, so I think now it's really just about getting the RFC merged, and then afterwards helping to get the @ember/test-helpers
PR (that I haven't opened yet) merged.
I think I've been in kinda a holding pattern here for some time, because Rob (the PR champion) hasn't had bandwidth for this, and I haven't had the time/focus to be a squeaky enough wheel 😄
@bendemboski Alright, this is now considered "Exploring" which, in new RFC stages, means that we're actively interested in. I think this looks pretty solid but I'm sure others will have some thoughts of their own so I'll work on getting some traction for this in an upcoming Ember Core meeting.
Great, glad to hear! It would take a little doing, but when the time is right I can definitely dust off and update my reference implementation library for DOM element descriptors and my @ember/test-helpers
, qunit-dom
, and fractal-page-object
PRs plugging those projects into the dom-element-descriptors
reference implementation library.
Keep me posted!
In case it helps, I revived and updated the various relevant projects/branches:
- dom-element-descriptors (implementation of this RFC)
-
@ember/test-helpers branch (shows how DOM helpers could use
dom-element-descriptors
) -
qunit-dom branch (shows how DOM assertions could use
dom-element-descriptors
) -
fractal-page-object branch (shows how page objects could use
dom-element-descriptors
)
@wagenet or anyone else -- any advice on how to get this moving forward again?
After discussion in the RFC Review Meeting we decided to move this to Final Comment Period.