ripplejs / ripple

A tiny foundation for building reactive views
http://ripplejs.github.io
1.28k stars 66 forks source link

0.6.0 API changes #41

Open anthonyshort opened 9 years ago

anthonyshort commented 9 years ago

As part of the changes I'm making to the API for 0.6, I thought I'd talk about some of the goals and things I want to fix:

Extract the data-binding engine

This will allow me to finish of the virtual DOM engine. When creating a view you would need to choose a rendering engine for the view.

var bindings = binder(template)
  .use(someDirective)
  .use(someFiltersAndShit);

var View = ripple()
  .engine(bindings);

The syntax for this is still a little in progress, but the code behind it works, it's just a matter of making it feel nice in the API.

Less confusing API

At the moment, you need to listen to events on the class and possibly on the instance.

View.on('mounted', function(view){
  view.foo = bar;
});

View.prototype.submit = function() {
  this.foo = bar;
};

This is a little confusing having them next to each other but with a different syntax. Obviously one is an event and the other is a handler. There are a couple of examples of how this could work here: https://gist.github.com/anthonyshort/2dbf56c398f320a4db61

Option 1:

At the moment, I think this might be the nicest solution:

var View = ripple({
  initialize: function(){},
  mounted: function(){
    this.foo = bar;
  }, 
  submit: function(){
    this.foo = bar;
  }
});

View
  .use(binding(template))
  .attr('foo');

And if a key an event lifecycle name we'd add it as a callback automatically. This is a little too much magic possibly, but it does create a nice abstraction and allows all the methods to be in place.

Option 2

We use the ripple() function to create View that we inherit from:

var View = ripple()
  .engine(bindings)
  .attr('firstName')
  .attr('lastName');

View.on('initialize', function(view){
  view.foo = bar;
});

module.exports = View.extend({
  onClick: function(event){
    console.log('clicked!'); 
  }
});

Again, I think this confuses things because there are two different ways to add 'methods' and each has a different context. We'd also be using inheritance.

Option 3

Using a handler or method function to add methods so that it follows the same syntax as the events:

View.on('initialize', function(view){
  view.foo = bar;
});

View.handler('onClick', function(view, event){
  view.foo = bar;
});

Allow for ES6 modules and classes

If we used Option 1 above, it could possibly allow us to have a nice syntax for using ES6 classes. It's not necessary, but it would be nice.

import ripple from 'ripple';
import binder from 'binder';
import template from './template';

var bindings = binder()
  .use(someDirective)
  .use(someFiltersAndShit);

class View {
  constructor() {
    super();
    console.log('initializing a view!');
  },
  onClick(event) {
    console.log('clicked!');
  }
}

export ripple(View)
  .engine(bindings(template))
  .attr('firstName')
  .attr('lastName');

With an alternative syntax using extends

var Base = ripple()
  .engine(bindings(template))
  .attr('firstName')
  .attr('lastName');

exports class View extends Base {
  initialize() {
    console.log('initializing a view!');
  },
  onClick(event) {
    console.log('clicked!');
  }
}

Option 1, at the very least, would allow us to do this:

var View = ripple({
  mounted() {
    this.foo = bar;
  }, 
  submit() {
    this.foo = bar;
  }
});
bmcmahen commented 9 years ago

I agree with option 1 seeming the most intuitive, with binding being a bit nicer that way too than the current situation.

anthonyshort commented 9 years ago

Currently where I'm at with it working with ES6 modules:

import ripple from "ripplejs/ripple";
import binding from "ripplejs/binding-engine";
export ViewModel;

// The class for the template handles things like returning the 
// template string/function, hooking up DOM functionality on mount/destroy,
// the handlers for user interaction with the DOM.
class View {
  constructor(attrs) {
    // do initialisation stuff here
    this.name = attrs.name || 'default name';
  }
  render() {
     // Using the basic, built-in templating. For bindings it would return a string
     // as normal, for virtual DOM it would return a virtual DOM tree.
    return '<div>' + this.name + '</div>';
  }
  onSubmit(event) {
    event.preventDefault();
    this.name = 'foo';
    this.submit();
  }
  onMount() {
    // will automatically get added as a 'mounted' callback
  }
  onDestroy() {
    // will automatically get called on 'destroyed' event
  }
}

// Create View wrapper. Setting the engine type here automatically
// uses the plugin for the engine type which adds methods to the view.
var ViewModel = ripple(View)
  .engine(binding); // binding, string, or virtual

// Define attributes that define the state of the view
// These work the same as models so plugins can get to the
// options for each attribute to add extra functionality.
ViewModel
  .attr('name', { required: true, type: 'string' })
  .attr('email', { required: true, type: 'email' })

// Add plugins that can add directives, helpers, etc.
ViewModel
  .use(emailTypePlugin)
  .use(someDirectives)

// Lifecycle events of the view. Whenever the view is created,
// destroyed, or added and removed from the DOM an event is fired.
ViewModel
  .on('create', function(view, attributes){});
  .on('mount', function(view, parent){});
  .on('unmount', function(view){});
bmcmahen commented 9 years ago

Looks great to me, although I wonder if it might just be best to make the virtual engine the default, with an optional overwrite. I suspect most people will want the virtual engine, and it saves a bit on boilerplate.

anthonyshort commented 9 years ago

Yeah a few people have mentioned that. I'll probably just include it to keep it simple. Depending on what the size is like I could just include all the template engines and you'd just choose it:

var ViewModel = ripple(View, { engine: 'virtual' })

with the virtual being the default.

ianstormtaylor commented 9 years ago

ViewModel

The names are confusing me a bit here. I'd personally want to avoid something called a ViewModel because it makes me think the abstraction is getting too high. I think it might make sense to call it Template and View instead if this is the direction we are going, so that the pieces themselves stay simple to reason about. I generally never know what a ViewModel does.

The .use() method on the ViewModel also gets more confusing because I can't tell how the View object is referenced or how I would get at it. I think keeping this to a single constructor is going to be best, since it keeps things simpler in everyone's minds.

Classes vs. Prototypes

I think I agree with @visionmedia though that I don't really see the benefit of using the ES6 classes here. Seems like we're using them just to be able to use them. Instead, it makes most sense if ripple() returns a class, whether it uses straight up .prototype or class internally shouldn't matter much.

The way I think about it is that I'm never really going to want to re-use this View/Template class anywhere except in this case. I think here the ES6 classes are actually inhibiting keeping the API simple (probably a fault in their design, since they're not the nicest things).

I still think the pure .prototype approach still feels cleanest to me:

var View = ripple(template);

View.prototype.expand = fn;
View.prototype.collapse = fn;

In most cases we probably won't even need methods on the .prototype since most of the logic will be handled straight in the template string/function itself since it's all reactive?

It seems like the constructor method from the classes should be implemented with a .on('construct' event, and the render method from the classes shouldn't actually exist at all because the binding should take care of that for us automatically?

Schemas

I think my ideal would be to also be able to pass in a straight schema object like we've been talking about with our User and Project models/resources on the frontend, so that we don't have to manually call .attr() on each one. Something like this:

var View = ripple(template, schema);

Where schema is a .yml file like:

attributes:
  name:
    type: string
    validators:
      - required
  email:
    type: string
    validators:
      - required
      - required

That gives us the benefit of being able to share these schemas easily across our models and views since we're going to be declaring these same properties all over the place for each view that takes a User or Project model.

Result

With all of that, it would convert the example above into:

import ripple from "ripplejs/ripple";
import {user} from "segmentio/schemas";
export View;

/**
 * Create our virtual DOM template.
 *
 * @return {String}
 */

function template() {
  return virtualDom();
}

/**
 * Initialize a `View` with a `template` and `schema`.
 * 
 * @param {Object} attrs
 * @return {View}
 */

var View = ripple(template, schema)
  .use(emailTypePlugin)
  .use(someDirectives);

/**
 * Submit handler.
 *
 * @param {Event} e
 */

View.prototype.onSubmit = function (e) {
  e.preventDefault();
  this.name = 'foo';
  this.submit();
};

/**
 * Mount handler. Will be automatically called when the view is
 * injected into the DOM.
 * 
 * @param {Element} parent
 */

View.prototype.onMount = function (parent) {
  // ...
};

/**
 * Destroy handler. Will be automatically called when the view is destroyed.
 */

View.prototype.onDestroy = function () {
  // ...
};

Basically the prototype keeps things simple as the only way to do things, using the onEvent pattern for method names, so that we get around the problem of having to bind events on the constructor like you mentioned.

Would be interesting to get @MatthewMueller's feedback since he probably has good ideas here.

tj commented 9 years ago

passing something in that already has a proto is still cool, at least then it supports ES6 class stuff too if anyone prefers that. I'd probably just stay away from requiring the need for super() etc. Inheritance is an anti-pattern IMO but we'll probably see a ton of that now yay design by committee

anthonyshort commented 9 years ago

Yeah inheritance is gross. Avoiding that. I'll only use the classes as a nicer way to create constructors. The whole prototype thing is ok, but it's super verbose and obscure. Classes are just a more obvious syntax for it.

Yeah ViewModel is a weird name. Like you said, it's more like a Template class that you're passing in to create a View.

But I do think it's more simple, since you're not dealing with events + prototype + static methods all on the same object. Plus, the big one, is that you know what object you're dealing with and the context when you get passed a template. It would always been an instance of one of those classes.

The class events, like View.on('create', fn), will only really be used by plugins, and it allows us to keep the initialization stuff in the class constructor without needing to have that as an event to keep things consistent.

As for the schema, I'd probably just make that a plugin to automatically add attributes:

View.use(schema(thing));

In most cases, it will just look like:

class View {
  constructor() {
    this.seconds = 0;
  }
  onMount() {
    this.setInterval(this.tick, 1000);
  }
  tick() {
    this.seconds++;
  }
  render() {
    return `<div>${this.seconds}</div>`;
  }
}

export ripple(View)
  .engine('string')
  .use(intervals())
  .attr('seconds', { type: 'number' });

Which you could easily make using a constructor function too.

tj commented 9 years ago

ripple(View) would make testing View on its own really easy too

ianstormtaylor commented 9 years ago

Gotcha, that makes sense.

What's that example look like if we were using the virtual dom instead? I assume we'd start using that by default everywhere for the most part?

Slash, is there any reason to use the intervals() plugin there since we already have the setInterval call in the onMount method?

And can the engine still be guessed automatically by us so that people don't have to set it?

anthonyshort commented 9 years ago

If the injecting the class/constructor into it seems weird, I'd probably almost rather just go the React-style to declare it. At least the syntax is more terse than prototypes and it's super noob-friendly.

It could actually pretty easily support both, but I'd rather just have one.

var View = ripple({

  /**
   * Set the initial state of the view
   *
   * @param {Object} attrs
   */

  initialize(attrs) {
    this.seconds = attrs.startTime || 0;
  }

  /**
   * Mount handler. Will be automatically called when the view is
   * injected into the DOM.
   * 
   * @param {Element} parent
   */

  onMount() {
    this.interval = setInterval(this.tick, 1000);
  }

  /**
   * Unmount handler. Will be automatically called when the view is
   * removed from the DOM.
   *
   * @param {Element} parent
   */

  onUnmount() {
    clearInterval(this.interval);
  }

  /**
   * Increment the timer
   */

  tick() {
    this.seconds++;
  }

  /**
   * Return the template used by the engine to 
   * render the view. This is called by the engine.
   */

  render() {
    return `<div>${this.seconds}</div>`;
  }

});

/**
 * Set the template engine
 */

View.engine('string');

/**
 * Use some plugins 
 */

View.use(intervals());
View.use(refs());
View.use(someDirectives());

/**
 * Attributes to observe
 */

View.attr('seconds', { type: 'number' });

/**
 * Export
 */

export default View;
ianstormtaylor commented 9 years ago

Actually I dig your idea for injecting the constructor now that I see that most of the logic on the "ViewModel" has gone away. Mostly curious to see if we can eliminate the last bit of logic that touches the ViewModel layer, because it's only confusing when the two layers have lots going on in each.

But I like your higher-level concept of basically just turning ripple() into glue between a model and a view. That's kinda why I like the idea of making a schema that can just be used, so that you'd have things like:

var Model = model(schema);
var View = ripple(template, Model);

But then you don't want to couple to a Model implementation, so instead:

var Model = model(schema);
var View = ripple(template, schema);

If that makes sense, and then easily use them together. In our case the model is actually just a resource object that has method that hit an HTTP API, and it doesn't actually store state, since that's now all stored in Ripple.

That's from a pseudocode standpoint though, since .attr() might be the better way to expose an API to add the schema. I'd just probably make like hades-ripple and hades-http that both take a standard schema and return an HTTP object and a View object.

anthonyshort commented 9 years ago

@ianstormtaylor Yeah I was thinking about the engine thing. It seems a little magic-y, but it could be good. I kinda like that you'd need to set it manually so it's super clear what it should return.

anthonyshort commented 9 years ago

Yeah it's basically just joining that View to the engine and observing properties on that view. You don't really need a model object though. It's just stored as a plain object in ripple, and when that object changes it just tells the engine.

The attributes on a view are pretty rarely actually tied directly to a real model's values. They get transformed somehow or joined together, and then there are properties that don't exist on the model but do store view state, things like hidden come to mind.

matthewmueller commented 9 years ago

I'm probably missing something, but I tend to prefer just extending the prototype rather than passing an object through.

var View = ripple(attrs);

/**
 * Set the initial state of the view
 *
 * @param {Object} attrs
 */

View.prototype.initialize = function(attrs) {
  this.seconds = attrs.startTime || 0;
}

/**
 * Mount handler. Will be automatically called when the view is
 * injected into the DOM.
 * 
 * @param {Element} parent
 */

View.prototype.onMount = function() {
  this.interval = setInterval(this.tick, 1000);
}

/**
 * Unmount handler. Will be automatically called when the view is
 * removed from the DOM.
 *
 * @param {Element} parent
 */

View.prototype.onUnmount = function() {
  clearInterval(this.interval);
}

/**
 * Increment the timer
 */

View.prototype.tick = function() {
  this.seconds++;
}

/**
 * Return the template used by the engine to 
 * render the view. This is called by the engine.
 */

View.prototype.render = function() {
  return "<div>${this.seconds}</div>";
}

I also think events should probably be complimentary for things like mounting, unmounting, initializing as you may want to tap into that stuff with plugins.

matthewmueller commented 9 years ago

p.s.- love the virtual dom direction you seem to be heading.

chrisbuttery commented 9 years ago

I'm digging Option 1. It seems to read better, in my eyes.