From 84ad27bccde4b241741802710ee1c97bc69df0d6 Mon Sep 17 00:00:00 2001 From: phil Date: Sat, 25 Jun 2016 10:56:10 -0400 Subject: [PATCH] Kill `emit` for normal prop callbacks add a semi-awkard api for calling back the props in the child component. TODO: - let the child component specify the propTypes that it will call any callback with - let the child specify whether providing the callback is optional or not (or just rely on propTypes? probably...) There are two levels toward ease-of-typing that this can move toward. Currently, you do: ```js Update({ cb: { onAdd: [task] }, }); ``` We need the wrap the payload in an array so that we can apply to the callback with whatever number of arguments are required. So we could hack this structure with only an array instead: ```js Update({ cb: ['onAdd', task], }); ``` Which is a little ugly and feels to string-y, but is maybe a little less error-prone on the array-wrapping We could go full react: ```js Update({ onAdd: [task], }); ``` which, damn, is sort of nice to type and read. But also... namespacing, magic, ugh ugh. I'm sticking with the explicit nested-object version for now. The array-wrapping could either be totally gross and allow an unwrapped payload for single-arg callbacks (which _is_ by far the most common case), or at least have a useful error message for an unwrapped payload that would be annoying, but easy to debug. --- Onnnnnn the parent side, using regular react-style prop callbacks is wonderful. Not having to import ChildEmit to switch over different emit payload types is way better. This feels really right. --- examples/constrained-counters/Counter.js | 11 ++++------ examples/constrained-counters/Parent.js | 4 ++-- examples/sum-counters/Counter.js | 6 +++--- examples/sum-counters/SummedPair.js | 4 ++-- examples/todomvc/Header.js | 2 +- examples/todomvc/Item.js | 14 ++++-------- examples/todomvc/TodoMVC.js | 21 +++++++++--------- spindle.js | 27 +++++++++++------------- 8 files changed, 39 insertions(+), 50 deletions(-) diff --git a/examples/constrained-counters/Counter.js b/examples/constrained-counters/Counter.js index d29ac57..979cb1f 100644 --- a/examples/constrained-counters/Counter.js +++ b/examples/constrained-counters/Counter.js @@ -5,9 +5,6 @@ import { Union, Maybe } from 'results'; import Spindle, { Update } from '../../spindle'; -const emitType = PropTypes.number.isRequired; - - const Model = Record({ value: 0, min: -Infinity, @@ -43,7 +40,7 @@ const propsUpdate = ({ min = -Infinity, max = Infinity }, model) => { const updated = constrain(model.merge({ min, max })); return Update({ model: updated, - emit: model.value, + cb: { onChange: [model.value] }, }); }; @@ -53,7 +50,7 @@ const update = (action, model) => Action.match(action, { const newModel = constrain(model.update('value', v => v + 1)); return Update({ model: newModel, - emit: newModel.value, + cb: { onChange: [newModel.value] }, }); }, @@ -61,7 +58,7 @@ const update = (action, model) => Action.match(action, { const newModel = constrain(model.update('value', v => v - 1)); return Update({ model: newModel, - emit: newModel.value, + cb: { onChange: [newModel.value] }, }); }, }); @@ -85,4 +82,4 @@ const view = (model, dispatch) => ( export default Spindle('Counter', - { Action, init, propsUpdate, update, modelType, view, emitType }); + { Action, init, propsUpdate, update, modelType, view }); diff --git a/examples/constrained-counters/Parent.js b/examples/constrained-counters/Parent.js index 6322e40..9af87fc 100644 --- a/examples/constrained-counters/Parent.js +++ b/examples/constrained-counters/Parent.js @@ -34,7 +34,7 @@ const view = (model, dispatch) => (

min

val

@@ -44,7 +44,7 @@ const view = (model, dispatch) => (

max

); diff --git a/examples/sum-counters/Counter.js b/examples/sum-counters/Counter.js index 6087273..77c8afc 100644 --- a/examples/sum-counters/Counter.js +++ b/examples/sum-counters/Counter.js @@ -11,7 +11,7 @@ const Model = Record({ const init = () => { const model = Model(); - return Update({ model, emit: model.value }); + return Update({ model, cb: { onChange: [model.value] } }); }; @@ -26,7 +26,7 @@ const update = (action, model) => Action.match(action, { const newModel = model.update('value', v => v + 1); return Update({ model: newModel, - emit: newModel.get('value'), + cb: { onChange: [newModel.get('value')] }, }); }, @@ -34,7 +34,7 @@ const update = (action, model) => Action.match(action, { const newModel = model.update('value', v => v - 1); return Update({ model: newModel, - emit: newModel.get('value'), + cb: { onChange: [newModel.get('value')] }, }); }, }); diff --git a/examples/sum-counters/SummedPair.js b/examples/sum-counters/SummedPair.js index fc51b3d..7f05a1a 100644 --- a/examples/sum-counters/SummedPair.js +++ b/examples/sum-counters/SummedPair.js @@ -33,8 +33,8 @@ const update = (action, model) => Action.match(action, { const view = (model, dispatch) => (

here are some counters:

- - + +

their sum is: {model.topValue + model.bottomValue}

); diff --git a/examples/todomvc/Header.js b/examples/todomvc/Header.js index 017856d..3c381d3 100644 --- a/examples/todomvc/Header.js +++ b/examples/todomvc/Header.js @@ -21,7 +21,7 @@ const update = (action, model) => Action.match(action, { KeyDown: e => { const task = e.target.value.trim(); if (e.keyCode === 13 && task !== '') { - return Update({ emit: task, model: '' }); + return Update({ cb: { onAdd: [task] }, model: '' }); } else { return Update(); } diff --git a/examples/todomvc/Item.js b/examples/todomvc/Item.js index 6532b0c..8d8794c 100644 --- a/examples/todomvc/Item.js +++ b/examples/todomvc/Item.js @@ -22,12 +22,6 @@ const Action = Union({ }); -export const Emit = Union({ - Destroy: null, - Save: null, -}); - - const init = ({ task }) => Update({ model: Model({ task }) }); @@ -38,13 +32,13 @@ const propsUpdate = ({ task }, model) => const update = (action, model) => Action.match(action, { Destroy: () => - Update({ emit: Emit.Destroy(model.task.id) }), + Update({ cb: { onDestroy: [model.task.id] } }), FinishEditing: () => { const task = model.task.set('title', model.editingValue); return Update({ model: model.merge({ task, editing: false }), - emit: Emit.Save(task), + cb: { onSave: [task] }, }); }, @@ -53,7 +47,7 @@ const update = (action, model) => Action.match(action, { const task = model.task.set('title', model.editingValue); return Update({ model: model.merge({ task, editing: false }), - emit: Emit.Save(task), + cb: { onSave: [task] }, }); } else { return Update(); @@ -73,7 +67,7 @@ const update = (action, model) => Action.match(action, { const task = model.task.update('completed', c => !c); return Update({ model: model.set('task', task), - emit: Emit.Save(task), + cb: { onSave: [task] }, }); }, }); diff --git a/examples/todomvc/TodoMVC.js b/examples/todomvc/TodoMVC.js index ea91a50..c053c0f 100644 --- a/examples/todomvc/TodoMVC.js +++ b/examples/todomvc/TodoMVC.js @@ -3,7 +3,7 @@ import Spindle, { Update } from '../../spindle'; import Immutable, { Record } from 'immutable'; import { Union } from 'results'; import Header from './Header'; -import Item, { Emit as ItemEmit } from './Item'; +import Item from './Item'; const Task = Record({ @@ -15,7 +15,8 @@ const Task = Record({ const Action = Union({ Add: null, - Change: null, + Save: null, + Destroy: null, ClearCompleted: null, MarkAllComplete: null, }); @@ -32,12 +33,11 @@ const update = (action, model) => Action.match(action, { return Update({ model: model.set(nextId, task) }); }, - Change: msg => ItemEmit.match(msg, { - Destroy: id => - Update({ model: model.delete(id) }), - Save: task => - Update({ model: model.set(task.get('id'), task) }), - }), + Destroy: id => + Update({ model: model.delete(id) }), + + Save: task => + Update({ model: model.set(task.get('id'), task) }), ClearCompleted: () => Update({ model: model.filter(task => !task.completed) }), @@ -51,7 +51,7 @@ const view = (model, dispatch) => { const incomplete = model.filter(task => !task.completed); return (
-
+
{model.size > 0 && [
{ {model.map(task => ( )).toArray()} diff --git a/spindle.js b/spindle.js index e95494b..1fedb3a 100644 --- a/spindle.js +++ b/spindle.js @@ -6,7 +6,7 @@ import { Union } from 'results'; const validUpdateKeys = { model: true, cmds: true, - emit: true, + cb: true, }; export function Update(stuff) { @@ -19,7 +19,7 @@ export function Update(stuff) { Object.keys(stuff).forEach(k => { if (typeof validUpdateKeys[k] === 'undefined') { throw new TypeError(`Unrecognized key \`${k}\` supplied to Update(). ` + - 'Valid keys: `model`, `emit`, and `cmds`.'); + `Valid keys: ${Object.keys(validUpdateKeys).join(', ')}`); } }); Object.assign(this, stuff); @@ -36,11 +36,9 @@ const assertType = (who, checker, value, name, source) => { const propsEq = (a, b) => { for (const k in a) { - if (a[k] !== b[k] && k !== 'onEmit') return false; + if (a[k] !== b[k]) return false; } - const aCount = Object.keys(a).length - !!a.onEmit; - const bCount = Object.keys(b).length - !!b.onEmit; - return aCount === bCount; + return Object.keys(a).length === Object.keys(b).length; }; @@ -144,7 +142,6 @@ export default function Spindle(name, { view = () => null, subscriptions = () => [], modelType = PropTypes.any, - emitType = PropTypes.any, propTypes: componentPropTypes = {}, }) { class Component extends React.Component { @@ -203,7 +200,7 @@ export default function Spindle(name, { `Did you forget to wrap a new model in \`Update({ model: ... })\`?`); } - const { model, cmds, emit } = update; + const { model, cmds, cb } = update; if (typeof model !== 'undefined') { assertType('model', modelType, model, name, source); @@ -217,9 +214,12 @@ export default function Spindle(name, { const subs = subscriptions(this._model); this.getSpindle().updateSubs(this, subs); - if (typeof emit !== 'undefined' && this.props.onEmit) { - assertType('emit', emitType, emit, name, source); - this.props.onEmit(emit); + if (typeof cb !== 'undefined') { + Object.keys(cb) + .filter(prop => + this.props[prop]) + .forEach(prop => + this.props[prop].apply(null, cb[prop])); } if (source === 'init') { @@ -242,10 +242,7 @@ export default function Spindle(name, { childContextTypes: { spindle: PropTypes.object, }, - propTypes: { - ...componentPropTypes, - onEmit: PropTypes.func, // optional - } + propTypes: componentPropTypes, }); return Component; };