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

Figure out a way to remove the requirement of the chain's data as the first argument #17

Open
balupton opened this issue Aug 3, 2014 · 2 comments

Comments

@balupton
Copy link
Member

balupton commented Aug 3, 2014

Let's take the set action:

require('chainy').require('log')
    .addExtension('set', 'action', function(value, newValue){
        return newValue
    })
    .create()
    .set('blah')
    .log()  // blah

It needs to accept that value argument, as there is no way to turn it off.

Unfortunately, the new args taskOption added in v1.5.0 of Chainy, won't cover it, as:

Chainy.require('log')
    .addExtension('set', {
        type: 'action',
        taskOptions: {
            args: []
        }
    }, function(value, newValue){
        return newValue
    })
    .create()
    .set('blah')
    .log()  // null

Will output null as no arguments would be received.


This is something that I was considering about when releasing v1.5.0, however, v1.5.0 is still and improvement and offers cool stuff, despite this oversight.


What we probably need to do, is add a new special argument, like Chainy.injectArgumentsAsArguments, and Chainy.injectArgumentsAsArgument, then change the default action args to [Chainy.injectChainDataAsArgument, Chainy.injectArgumentsAsArguments].

This should maintain a lot of backwards compat, introduce some power, and accomplish the use case. With args: [] being changed to args: [Chainy.injectArgumentsAsArguments] instead.


The other option is to allow for args: false to be a special case, which means don't apply the action default arguments, but still apply the task options default arguments.


Another approach, which is similar to the args: false suggestion, which I initially coded before v1.5.0 but then dropped, was a new task extension type, and a new chain.task(method, opts). That will be the same as the action extension, but without the argument requirement. However, after implementation it proved to be an anti-pattern to a sort, as the user should have control over the argument insertion for the most part, and could cause confusion for implementors about which type they want. However, considering v1.5.0 is out with new args handling, this could be worth reconsideration as the anti-pattern may be subverted by the new args handling.


Need to consider this and weigh it up.

balupton added a commit to chainyjs/chainy-core that referenced this issue Aug 4, 2014
@balupton
Copy link
Member Author

balupton commented Aug 4, 2014

So I've added the special argument options, as well as added the ability to send user arguments to .action. It works well: chainyjs/chainy-core@4995448

Allowing:

require('chainy').require('log')
    .addExtension('set', {
        type: 'action',
        taskOptions: {
            args: [Chainy.injectArgumentsAsArguments]
        }
    }, function(newValue){
        return newValue
    })
    .create()
    .set('blah')
    .log()  // blah

This leaves the question of whether or not the task extension type makes sense. This is something I will need to evaluate in practice.

The task extension type would change would change the above to:

require('chainy').require('log')
    .addExtension('set', 'task', function(newValue){
        return newValue
    })
    .create()
    .set('blah')
    .log()  // blah

The problem I see is that this could be an anti-pattern against the chain waterfall flow dynamic that chainy does so well.


An alternative could be if you call an extension with an underscore at the start, it behaves as a task. This seems dangerous with the new work in that commit. But would allow more control to the user rather than the extension developer.


The task dynamic has some interesting abilities.

@balupton
Copy link
Member Author

So I've been thinking about this a lot the past few months, and I think I've come to the following solution:

var Chainy = require('chainy'),
  $ = Chainy.$, // alias for injectChainData
  $$ = Chainy.$$ // alias for injectExpandedChainData

Chainy.create()
  .addExtension('set', 'task', function(value){return value})
  .addExtension('map', 'task', function(value, iterator){return value.map(iterator)})
  .set([5, 10])
  .map($, function(value){return value*2})
  .log($)  // [10, 20]
  .log($$)  // 10, 20

That much is definite, however I need to do up more examples of this, especially around the complexities of say redis database integration and usage.

Another question is (which the redis stuff will showcase), is how does one indicate whether or not we care about over-riding the chain's data. This could be solved via a few ways, e.g.

// Via a special argument
Chainy.create()
  .set(5)
  .set(10, Chainy.dontSetChainData)
  .log($)  // 5

// Via a method alias
Chainy.create()
  .set(5)
  ._set(10)
  .log($)  // 5

// Via a store plugin to reset the chains data when it is over-written, with last option
Chainy.create()
  .set(5).store()
  .set(10).restore()
  .log($)  // 5

// Via a store plugin to reset the chains data when it is over-written, with naming
Chainy.create()
  .set(5).store('five')
  .set(10).restore('five')
  .log($)  // 5

// Via a store plugin as the only way to update chain data
Chainy.create()
  .set(5).store()
  .set(10)
  .log($)  // 5

// Via store plugin as the only way to update chain data with $() helper, but can do storage with .store() helper, features names too
Chainy.create()
  .set(5).$('five')
  .set(10).store('ten')
  .log($)  // 5
  .restore('ten')
  .log($)  // 10
  .restore('five')
  .log($)  // 5

Remaining questions:

  • What if I want to reshuffle a chain data's array?

/cc @JedWatson keen to get your feedback on this comment's proposal, it could really change the way we write javascript code

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

No branches or pull requests

1 participant