Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promise builder interface #44

Closed
wants to merge 1 commit into from
Closed

Promise builder interface #44

wants to merge 1 commit into from

Conversation

joshski
Copy link
Member

@joshski joshski commented Jul 24, 2016

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
Copy link
Member

dereke commented Jul 25, 2016

love it

@dereke
Copy link
Member

dereke commented Dec 2, 2016

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

@joshski
Copy link
Member Author

joshski commented Dec 2, 2016

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
Copy link
Member Author

joshski commented Dec 2, 2016

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
Copy link
Member Author

joshski commented Feb 13, 2017

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
Copy link
Member Author

joshski commented Feb 13, 2017

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.

@joshski joshski closed this Feb 13, 2017
@joshski joshski deleted the builder-api branch February 13, 2017 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants