Skip to content

Commit

Permalink
Kill emit for normal prop callbacks
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
uniphil committed Jun 25, 2016
1 parent 0b71059 commit 84ad27b
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 50 deletions.
11 changes: 4 additions & 7 deletions examples/constrained-counters/Counter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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] },
});
};

Expand All @@ -53,15 +50,15 @@ 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] },
});
},

Decrement: () => {
const newModel = constrain(model.update('value', v => v - 1));
return Update({
model: newModel,
emit: newModel.value,
cb: { onChange: [newModel.value] },
});
},
});
Expand All @@ -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 });
4 changes: 2 additions & 2 deletions examples/constrained-counters/Parent.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const view = (model, dispatch) => (
<div>
<p>min</p>
<Counter
onEmit={dispatch.SetMin}
onChange={dispatch.SetMin}
max={model.get('max')} />

<p>val</p>
Expand All @@ -44,7 +44,7 @@ const view = (model, dispatch) => (

<p>max</p>
<Counter
onEmit={dispatch.SetMax}
onChange={dispatch.SetMax}
min={model.get('min')} />
</div>
);
Expand Down
6 changes: 3 additions & 3 deletions examples/sum-counters/Counter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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] } });
};


Expand All @@ -26,15 +26,15 @@ 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')] },
});
},

Decrement: () => {
const newModel = model.update('value', v => v - 1);
return Update({
model: newModel,
emit: newModel.get('value'),
cb: { onChange: [newModel.get('value')] },
});
},
});
Expand Down
4 changes: 2 additions & 2 deletions examples/sum-counters/SummedPair.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ const update = (action, model) => Action.match(action, {
const view = (model, dispatch) => (
<div>
<p>here are some counters:</p>
<Counter onEmit={dispatch.TopValue} />
<Counter onEmit={dispatch.BottomValue} />
<Counter onChange={dispatch.TopValue} />
<Counter onChange={dispatch.BottomValue} />
<p>their sum is: {model.topValue + model.bottomValue}</p>
</div>
);
Expand Down
2 changes: 1 addition & 1 deletion examples/todomvc/Header.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
14 changes: 4 additions & 10 deletions examples/todomvc/Item.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ const Action = Union({
});


export const Emit = Union({
Destroy: null,
Save: null,
});


const init = ({ task }) =>
Update({ model: Model({ task }) });

Expand All @@ -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] },
});
},

Expand All @@ -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();
Expand All @@ -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] },
});
},
});
Expand Down
21 changes: 11 additions & 10 deletions examples/todomvc/TodoMVC.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -15,7 +15,8 @@ const Task = Record({

const Action = Union({
Add: null,
Change: null,
Save: null,
Destroy: null,
ClearCompleted: null,
MarkAllComplete: null,
});
Expand All @@ -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) }),
Expand All @@ -51,7 +51,7 @@ const view = (model, dispatch) => {
const incomplete = model.filter(task => !task.completed);
return (
<section className="todoapp">
<Header onEmit={dispatch.Add} />
<Header onAdd={dispatch.Add} />
{model.size > 0 && [
<section key="main" className="main">
<input
Expand All @@ -65,7 +65,8 @@ const view = (model, dispatch) => {
{model.map(task => (
<Item
key={task.id}
onEmit={dispatch.Change}
onSave={dispatch.Save}
onDestroy={dispatch.Destroy}
task={task} />
)).toArray()}
</ul>
Expand Down
27 changes: 12 additions & 15 deletions spindle.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Union } from 'results';
const validUpdateKeys = {
model: true,
cmds: true,
emit: true,
cb: true,
};

export function Update(stuff) {
Expand All @@ -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);
Expand All @@ -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;
};


Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand All @@ -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') {
Expand All @@ -242,10 +242,7 @@ export default function Spindle(name, {
childContextTypes: {
spindle: PropTypes.object,
},
propTypes: {
...componentPropTypes,
onEmit: PropTypes.func, // optional
}
propTypes: componentPropTypes,
});
return Component;
};
Expand Down

0 comments on commit 84ad27b

Please sign in to comment.