featurist / browser-monkey

Reliable DOM testing
https://browsermonkey.org
53 stars 6 forks source link

Promise builder interface #44

Closed joshski closed 7 years ago

joshski commented 8 years ago

Not ready to merge, I'm just experimenting with an idea I posted.

This adds a new promise builder interface to every scope, via scope.builder()

The builder interface collapses a chain of scope->method->promise->then->scope->method calls into a chain of scope->method->method. For example:

scope.click('Happy').then(function() { return scope.click('Days'); }).then(...)

...can be written as:

scope.builder().click('Happy').click('Days').then(...)

Is this better? worth it?

dereke commented 8 years ago

love it

dereke commented 7 years ago

@joshski what do we need to do to move this forward?

joshski commented 7 years ago

Well, we could merge it and see if people use it. But IMO they aren't that likely to if it isn't the "main" interface. If we like this style in general (not confusing to mix up promises and fluent interfaces?) then perhaps we should not hang it off .builder() but make every scope a builder?

joshski commented 7 years ago

This is, of course, a solution to the general problem of promise-callback-hell :)

So perhaps the question is much about whether we should just wait for async/await? We have similar sugar in the API already...

Which of these do we like the best?

address.fill([
  {name: 'street',  action: 'typeIn', options: {text: 'Monkey St'}},
  {name: 'city',    action: 'typeIn', options: {text: 'Browserville'}},
  {name: 'country', action: 'select', options: {text: 'Monkey Island'}},
]);
await address.street().typeIn('Monkey St')
await address.city().typeIn('Browserville')
await address.country().select('Monkey Island')
await address
  .typeInto('Street', 'Monkey St')
  .typeInto('City', 'Browserville')
  .select('Country', 'Monkey Island')

Although they aren't mutually exclusive (and we can continue to support them all) it would be good to have some consensus!

I like the last one personally ;)

joshski commented 7 years ago

I tried to make this work implicitly i.e. where every component is also a promise. I couldn't get it working, which is a shame. But I think we should merge this, see if people use it, and consider adding some mechanism whereby the promises returned by components are also instances of those components, but probably not by using promise-builder IMO.

joshski commented 7 years ago

On second thoughts, I really don't like the fact that you have to call builder() on a component to get the builder interface. Let's not merge this for now, if you want to use this chain style, just use promise-builder in your project.