vvo / chainit

Turn an asynchronous JavaScript api into an asynchronous chainable JavaScript api
Other
12 stars 4 forks source link

skip functions #5

Closed christian-bromann closed 10 years ago

christian-bromann commented 10 years ago

Hi

I need a functionality for WebdriverJS that makes it possible to skip a bunch of tasks. As described in camme/webdriverjs#137 it would be awesome to do something like this:

client
  .command() // produces an error
  .command() // will be skipped because of previous error
  .onError(function() {...}) // until here (executed if one of the previous two callbacks were on error)
  .command() // will be executed again
  .command()

Take a look into my changes, I hope you like the way I tried to implement it. Basically you just have to determine a function that stops the queue from skipping. I called this function addNonSkippableFunction - it is like adding a new function to the API with an additional flag that tells ChainIt that this function should not be skipped. Here is a simple example (I added this one in the Readme.md):

var obj = new MyChainApi();

// define function that stops skip process
chainit.addNonSkippableFunction(obj,'stopSkiping');

obj
  .method1()
  .call(function() {
    // start skipping
    chainit.skip(obj);
  })
  .method1() // will be skipped
  .method2() // will be skipped
  .stopSkiping(); // stops skipping
  .method1() // will be executed again

What do you think?

Cheers

christian-bromann commented 10 years ago

Before you merge this please let me add a skipOnce and a skipMultiple:

obj
  .method1() // will be executed
  .call(function() {
    chainit.skipOnce(obj);
  })
  .method2() // will be skipped
  .method2() // will be executed again

and

obj
  .method1() // will be executed
  .call(function() {
    chainit.skipMultiple(obj,3); // skip upcoming 3 tasks
  })
  .method2() // will be skipped
  .method1() // will be skipped
  .method1() // will be skipped
  .method2() // will be executed again

This could be useful as well.

vvo commented 10 years ago

@christian-bromann If it does helps webdriverjs I will be happy to merge this but when I look at addNonSkippableFunction I can't help but think that this is not right at all.

If you need to skip functions based on a result, why don't you just:

obj
  .func
  .func2(function(err, res) {
    if (res == smthg) {
      return; 
    }

    this
      .otherfn
      .otherfn2
...
  })

?

vvo commented 10 years ago

As I said elsewhere, I think we may hit the limit with our chained API here is a super comment from @sebv on google an queue based chain APIS:

The queue based chain is a bad pattern when async call are involved, as soon as you try to do something > complex, or compose a bit, you end up having to manually halt the queue and insert tasks manually (But ?> maybe NightWatch will prove me wrong). We use to have a chain api like this in WD.js and there were so > many issues, that it led me to reimplement the chaining using promises.

on https://groups.google.com/forum/#!msg/appium-discuss/m8ypnn6lhLc/jDcugkzA3B8J

christian-bromann commented 10 years ago

Maybe queue based chain APIs hit limits at some point but I would love to try out how far it goes. Let me add the other two skip functions and then I would be pleased if this PR would make it into chainit.

vvo commented 10 years ago

Yes I will merge it but I do really think that it complexify a lot the API for something that is already possible by using simple callbacks + ifs. What do you think?

vvo commented 10 years ago

closing, think we do not need it right now, what do you think?