[Suggestions] Some syntactic sugar and extensions

[phanan]

So I've been playing around with the lib for a while and often find myself wishing for some helpers/wrappers that feel a little bit more fluent e.g.:

• Wrappers around event triggers i.e. wrapper.find(Foo).click() as a syntactic sugar for wrapper.find(Foo).trigger('click'). Maybe we can even go as far as wrapper.click(Foo)?
• contains(Foo, Bar, Baz)
• containsAny(Foo, Bar, Baz)
• has() as an alias for contains()
• id() for attributes().id() @robcresswell
• more to be added

What do you guys think?

[lmiller1990]

If you do wrapper.trigger('click'), that already works (triggers on component root element).

Can you explain contains a little more and give an example of what you'd want to write? At the moment I write expect(wrapper.findAll(ChannelUsersModalUser)).toHaveLength(2) which seems okay.

I think has could be added.

[phanan]

All but contains(Foo, Bar) and containsAny(Foo, Bar) are already there. What I’m suggesting are syntactic sugar or wrappers to make the exp better.

…

On Thu, Dec 21, 2017 at 01:19 Lachlan ***@***.***> wrote: If you do wrapper.trigger('click'), that already works (triggers on component root element). Can you explain contains a little more and give an example of what you'd want to write? At the moment I write expect(wrapper.findAll(ChannelUsersModalUser)).toHaveLength(2) which seems okay. I think has could be added. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#285 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHrt0kO9Aa9ns8FgqpDXnBztrNxlnTKCks5tCaQigaJpZM4RJGaq> .

[phanan]

On the other hand, I intentionally put “has” at the bottom of the list because I don’t have very strong opinion about it — when I think it’s a good alias, I can still live without it.

…

On Thu, Dec 21, 2017 at 09:01 An Phan ***@***.***> wrote: All but contains(Foo, Bar) and containsAny(Foo, Bar) are already there. What I’m suggesting are syntactic sugar or wrappers to make the exp better. On Thu, Dec 21, 2017 at 01:19 Lachlan ***@***.***> wrote: > If you do wrapper.trigger('click'), that already works (triggers on > component root element). > > Can you explain contains a little more and give an example of what you'd > want to write? At the moment I write > expect(wrapper.findAll(ChannelUsersModalUser)).toHaveLength(2) which > seems okay. > > I think has could be added. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#285 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AHrt0kO9Aa9ns8FgqpDXnBztrNxlnTKCks5tCaQigaJpZM4RJGaq> > . >

[38elements]

IMHO,
Since I do not like alias, I hope contains is removed if has is added.

[lmiller1990]

I agree with @38elements - if has is more popular, we should depreciate contains. Aliases just add more API to remember. People can add them to their own projects easily enough.

I thought about this a bit more and I'm generally against this kinds of things - I think being explicit and easily understood in your tests is a worth the extra typing (or just make a snippet, most IDE/editors have that support).

[phanan]

Aliases don’t add more APIs to remember. If anything, quite the opposite – they make things easier because _we don’t have to remember_. For example, top off your head can you remember which of lodash’s `extends` and `assigns` is an alias of which? Yet we don’t care, and just naturally use any of them. Libraries like PHPUnit, mockery, sinon etc. also have nice syntactic sugars e.g. `onFirstCall()`, `onSecondCall()`, `receiveOnce` etc whose functionalities can already be covered by other "original" methods, and those are what make the difference – I can't think of, or use, anything mockery does that normal PHPUnit mock can't, but I still pick mockery any day simply because the syntax is much more fluent. While I do understand why people are against aliases (I’m among them, trust me), there are some that I do believe would make our lives easier and a framework/lib more fluent/friendly, otherwise I wouldn’t be suggesting them.

…

On Fri, Dec 22, 2017 at 06:54 Lachlan ***@***.***> wrote: I agree with @38elements <https://github.com/38elements> - if has is more popular, we should depreciate contains. Aliases just add more API to remember. People can add them to their own projects easily enough. I thought about this a bit more and I'm generally against this kinds of things - I think being explicit and easily understood in your tests is a worth the extra typing (or just make a snippet, most IDE/editors have that support). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#285 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHrt0l4Cbo2VJoESxR2L4VV5aQO1EE-Vks5tC0QwgaJpZM4RJGaq> .

[robcresswell]

If I might add one more to the discussion; we currently have .classes() and .attributes().id. .id() would be a nice enhancement. It seems logical to me, at least. When I was first using the lib I just assumed it would already exist.

[eddyerburgh]

@robcresswell You can get the id value using the attributes method:

wrapper.attributes().id

[robcresswell]

@eddyerburgh Yep I know, I put that in my original comment; it was just a "sugar" suggestion.

[phanan]

@robcresswell Can't say I'm using id myself (generally using IDs in components is a bad idea), but I'll add that to the list.

[eddyerburgh]

(Sorry @robcresswell, I didn't see that extra id)

My opinion is that we should avoiding adding direct aliases, like has.

The API list is already large. Adding aliases will make it more difficult to browse.

I'm also not fond of adding helper methods like id, or click, that can be achieved easily with existing methods.

The more methods there are, the more maintenance there is, and the more opportunity there is for methods to get out of sync.

That said, there are some good suggestions in this thread, like contains(Foo, Bar, Baz).

I'm going to close this issue as there are too many discussion points. It will be easier to discuss these feature requests if each feature request has its own issue.

I'll start a new issue about extending contains to support multiple params. If anyone wants to continue discussing other suggestions, please create an issue.

[phanan]

@eddyerburgh While I understand and tend to agree with some of your reasons, not adding APIs because the list is already large or because there will be more maintenance is IMO not a strong enough argument.

• If the list is large, and people still find it lacking, then it's a problem needed to be solved right there
• Maintenance is obviously an issue, but should never be used to negate good addons, or a library will never grow.
• The test suite should cover methods getting out of sync issue. It it doesn't – well, we have a bigger problem to worry about.

That said, I do agree that there are too many discussion points – my intention was similar to a wishlist. Will create issues/PRs once I have time i.e. back from vacation.

[robcresswell]

@phanan I tend to use id for singular components; top level views, or side navs, for example. Where they will actually be unique; and then use classes for reusable components, so I'd disagree that they are "a bad idea" - I think that's a far too broad / generic statement.

@eddyerburgh That approach is reasonable, I can understand wanting to keep maintenance low and the API clean. Thanks for weighing in.