Boilerplate reduction for v0.8.0

[dobrite]

Related to #40

If a proxy is used between the injected model/view/intent then there is no need to declare an interface upfront. The flip side is that the observables hanging off the model/view/intent must be accessed through a getter function and a stringed arg.

Example from the README:

var HelloModel = Cycle.createModel(function (intent) {
  return { name$: intent.get('changeName$').startWith('') };
});

Modified data-flow-node.js:

function P() { // Proxy is ES6 reserved
  this.proxiedProps = {};
  this.get = function (streamKey) {
    if (this.proxiedProps[streamKey] === undefined) {
      this.proxiedProps[streamKey] = new Rx.Subject();
    }
    return this.proxiedProps[streamKey];
  }
}

function DataFlowNode() {
  var args = Array.prototype.slice.call(arguments);
  var definitionFn = args.pop();
  if (typeof definitionFn !== 'function') {
    throw new Error('DataFlowNode expects the definitionFn as the last argument.');
  }
  var proxies = [];
  for (var i = 0; i < definitionFn.length; i++) {
    proxies[i] = new P();
  }
  var wasInjected = false;
  var output = definitionFn.apply(this, proxies);
  checkOutputObject(output);
  copyProperties(output, this);
  this.inject = function injectIntoDataFlowNode() {
    if (wasInjected) {
      console.warn('DataFlowNode has already been injected an input.');
    }
    for (var i = 0; i < arguments.length; i++) {
      replicateAll(arguments[i], proxies[i].proxiedProps);
    }
    wasInjected = true;
  };
  this.clone = function () {
    // TODO this is broken
    return DataFlowNode.apply({}, interfaces.concat([definitionFn]));
  };
  return this;
}

ES6 Proxy would be perfect for this, but they are very much a WIP.

One idea is to default to loose mode (as above) with an opt in to strict mode (as it is now) by passing in interface string arrays. This opt-int strict checking is how React handles it with their propTypes. I found myself constantly making errors during the writing and refactoring of a toy cycle app.

I'm somewhat on the fence with the need for the .get (it is ugly in it's own way) and the stringed stream arg, but those massive interface arrays in the TodoMVC are scary. It would be awesome to couple this with the removal of the events view property as outlined in #40. That would make the README example app really nice and welcoming:

var Cycle = require('cyclejs');
var h = Cycle.h;

var HelloModel = Cycle.createModel(function (intent) {
  return { name$: intent.get('changeName$').startWith('') };
});

var HelloView = Cycle.createView(function (model) {
  return {
    vtree$: model.get('name$')
      .map(function (name) {
        return h('div', {}, [
          h('label', 'Name:'),
          h('input', {
            'attributes': {'type': 'text'},
            'ev-input': 'inputText$'
          }),
          h('h1', 'Hello ' + name)
        ]);
      })
  };
});

var HelloIntent = Cycle.createIntent(function (view) {
  return {
    changeName$: view.get('inputText$').map(function (ev) { return ev.target.value; })
  };
});

Cycle.createRenderer('.js-container').inject(HelloView);
Cycle.circularInject(HelloModel, HelloView, HelloIntent);

I'm probably missing something obvious here that is a deal-breaker. Also, the explicit interface declaration was likely a purposeful design decision. In that case please feel free to ignore my suggestion!

🍻

[staltz]

Wow, I really like your suggestion. Thanks! I hear you about boilerplate problems, and I'd like to reduce it as much as possible (but avoiding an API that makes the framework feel too "magical").

I found myself constantly making errors during the writing and refactoring of a toy cycle app.

That's a very important problem to solve.

those massive interface arrays in the TodoMVC are scary.

Yes they are.

I need to give some thought to your implementation, and also to the overall API design. My intuition is to lean towards your suggestion, since JS is after all a dynamically typed language, where we are used to code without specifying types or contracts. Interfaces in Cycle are a bit against that. I'll toy around with this suggestion, and comment back here soonish. 👍

[staltz]

Commits f9e733a and a52db82 are implementing this. Boilerplate is being succesfully removed. 👍

The README example would be:

var HelloModel = Cycle.createModel(function (intent) {
  return {name$: intent.get('changeName$').startWith('')};
});

var HelloView = Cycle.createView(function (model) {
  return {
    vtree$: model.get('name$')
      .map(function (name) {
        return h('div', {}, [
          h('label', 'Name:'),
          h('input', {
            'attributes': {'type': 'text'},
            'ev-input': 'inputText$'
          }),
          h('hr'),
          h('h1', 'Hello ' + name)
        ]);
      })
  };
});

var HelloIntent = Cycle.createIntent(function (view) {
  return {
    changeName$: view.get('inputText$')
      .map(function (ev) { return ev.target.value; })
  };
});

Cycle.circularInject(HelloModel, HelloView, HelloIntent);
Cycle.createRenderer('.js-container').inject(HelloView);

I'll still do some commits on lessboilerplate branch, to update the docs and README, etc. I might join these changes with also #58, removing dom-delegator, before releasing v0.8.0 which should have significantly slimmer API.

Thanks @dobrite !

[staltz]

And I've been investigating if I could use ES6 Proxy or something equivalent (maybe a polyfill), but there are no hopes yet. We need to wait for ES6 implemented in browsers. Only then can we make another version of Cycle without the use of get().

[dobrite]

Wow, absolutely amazing! I looked through the 2 commits and what I saw looked good (but I'll admit most of it was over my head). I'll pull the branch and try it out with my toy app and see how it goes. So excited to delete those arrays. 🍻

I too looked for polyfills, googled, and checked SO and nothing turned up. It would be lovely to remove the gets eventually, but that will likely take a long while.

[dobrite]

Toy app working! 👍

[dobrite]

So my app hits a brick wall after a few minutes of running (~1.5 gb of ram allocated). Heap Allocations in chrome turns up ReplaySubjects. No idea if this means anything or is just a red herring:

master

lessboilerplate

Perhaps this? No idea what it is but it looked a little 🐟 on my read through.

[secobarbital]

Would ES5 getters/setters further clean up the syntax? Or is that still too much to expect from the supported browsers?

[secobarbital]

Oh never mind, I guess you'd still have to know the props up-front to define getters/setters.

[staltz]

So my app hits a brick wall after a few minutes of running (~1.5 gb of ram allocated).

I'll try to fix it before merging to master. Can you share the code, or tell me the use case, so I can reproduce it?

Would ES5 getters/setters further clean up the syntax? Or is that still too much to expect from the supported browsers? Oh never mind, I guess you'd still have to know the props up-front to define getters/setters.

Yeah I looked into that, and it won't be enough. Has to be ES6 Proxy.

[dobrite]

The custom-element example exhibits the same issue although slower. Pretty sure all apps will at some rate. I copied the ticker a bunch of times to get it to happen faster. In Chrome I used Profiles -> Record Heap Allocations to generate those screenshots above.

[staltz]

@dobrite, I fixed the memory leak. Turned out to be just the wrong way of using the replay() operator. replay(null, 1) and not replay(1). ebc3c1e

Check if it solves your issue.

[staltz]

Commit aad0dee removes circularInject. Now it is possible to tie together Model-View-Intent using just

Intent.inject(View).inject(Model).inject(Intent);

a.inject(b) returns b, that's why this is possible. The result is actually slightly more boilerplate compared to circularInject, but I prefer this API since it's more clear, more explicit, not less flexible, and you can still easily build your own circularInject function based on this new idiom. I prefer to reduce the number of primitive framework functions/helpers, if it doesn't introduce tedious boilerplate.

[dobrite]

Memory leak looks good on my end! 👍

I think the change to remove circularInject is a nice one. Previously it seemed magical. Now it just seems like any other (chainable) method.

[staltz]

Merged to master and considered done.