mainmatter/ember-test-selectors

Add support for native class components

alexdiliberto opened this issue · 8 comments

I'm not seeing the expected behavior when binding the data-test-* ID within the Javascript file when using Native Classes

Taking your existing README.md example:

export default Ember.Component({
  comment: null,
  'data-test-comment-id': Ember.computed.readOnly('comment.id'),
});

Now convert to use native class syntax and observe that data-test-comment-id does not exist anymore.

Here is a very simple Ember Twiddle, inspect the DOM and you notice data-test-my-component attribute does not exist

https://ember-twiddle.com/7cf08b835afbcbba34f761bcf41930bc

@alexdiliberto any ideas how to fix this?

It would be pretty nice if ember-test-selectors shipped a class decorator that allowed you to do something like

import { testSelector } from 'ember-test-selectors';

@testSelector('comment-id')
class WhateverComponent extends Component {
}

I'm not sure what the syntax would look like if you wanted to bind to a specific value, but this would help get around the current issues.

Maybe the decorator API from ember-css-modules could be helpful for inspiration

https://github.com/salsify/ember-css-modules#native-es6-class-syntax

@alexlafroscia what would that do?

In this example, set up the data-test-comment-id attribute on the outer element, and be stripped out of the build in production using babel-plugin-filter-imports or something

I'm not convinced. How to tagName: '' components work with native classes? Since the mid-term goal is to make tagName: '' the default it would be easy then to use <div data-test-comment-id>the content</div> in the template when needed. 🤔

I mean, you can pretty easily do tagName: '' with native class components:

import { tagName } from '@ember-decorators/component';

@tagName('')
class MyComponents extends Component {

}

I'm not sure why tagName and the ES6 classes would be entangled; while the default behavior in the future will be not having an outer element, I don't think that removes the need for a solution here

I don't think that removes the need for a solution here

I personally don't see the need here and won't put time into it, but feel free to come up with a PR. The constraints are that it needs to behave quite close to the current solution and be entirely optimized out in prod builds.

I'll close this - essentially dataAttributeBindings (which ETS adds all data-test- attributes to automatically) is only necessary as currently component templates define inner HTML rather than outer so that dataAttributeBindings is the only way to get the data-test attributes on the component's outer element. With the new component model where templates define the component's outer HTML that is no longer necessary and the data-test attribute can be added to the component's template directly.

In general, I'd advice against using a fixed data-test- attribute per component that is defined inside the component (be it the component's .js or .hbs) as that makes the tests using the attribute depend on internals of the component and the fact that a specific component is used. What you should do instead is pass data-test- attributes into the component from the context in that the component is invoked - after all, you would usually want to use different data-test- attributes for different invokations of the same component:

<Button data-test-delete-user-button>Delete</Button>
…
<Button data-test-save-record-button>Save</Button>