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

WIP Selectors #22

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,23 @@

## Unreleased

- Encourage passing the `props` object as the first argument to components.

Components are just functions so this isn't mandatory and you can still
define arguments as you see fit. The pattern of passing `props` makes
composition easier, encourages code which is more readable and decouples
the implementation of the component from the action of connecting it to the
store. From now on, connecting will only work for components with zero
arity or which take an object as the first argument.

- `connect` now takes a selector as an optional second argument.

You can now pass an optional selector function to `connect`. It will be
passed the `state` and should return an object which will be merged with
the component's `props` using `Object.assign({}, props, substate)`.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of first props then substate mimics what Redux does. I've wondered why they implement it this way. substate, props would allow overriding the props from state which sounds like it could be useful sometimes.

Copy link
Contributor

@bcruddy bcruddy Sep 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely think this makes more sense but I am curious as to what trade off the redux team made when they designed it the other way. There has to be a reason behind it.


connect(FooComponent, state => state.foo);

- Filter out booleans in the html helper.

Previously only `null` and `undefined` were filtered out. Now both `true`
Expand All @@ -22,6 +39,17 @@

html`${items.length > 0 && ItemList()}`

- Add an optional `combineReducers` module.

The `combineReducers` function can be used to hand off state changes to
smaller reducers. Each reducer takes care of one sub-tree of the state and
doesn't have access to the other parts.

`combineReducers` takes an object of `{name: reducer}` as the only
argument. The keys of the object will be used as top-level names in the
resulting state tree and the values will be set to the return values of
each of the reducers.


## 0.1.1 (September 12, 2017)

Expand Down
10 changes: 10 additions & 0 deletions combineReducers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export default function combineReducers(reducers) {
return function(state = {}, action, args) {
return Object.entries(reducers).reduce(
(acc, [name, reducer]) => Object.assign(acc, {
[name]: reducer(state[name], action, args),
}),
state
);
}
}
5 changes: 2 additions & 3 deletions example01/ActiveList.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ import { connect } from "./store";
import ActiveTask from "./ActiveTask";
import TaskInput from "./TaskInput";

function ActiveList(state) {
const { tasks } = state;
function ActiveList({tasks}) {
return html`
<h2>My Active Tasks</h2>
<ul>
${tasks.map(ActiveTask)}
${tasks.map((task, index) => ActiveTask({task, index}))}
<li>
${TaskInput()}
</li>
Expand Down
4 changes: 2 additions & 2 deletions example01/ActiveTask.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import html from "../index";

export default function ActiveTask(text, index) {
export default function ActiveTask({task, index}) {
return html`
<li>
${text}
${task}
<button
onclick="dispatch('COMPLETE_TASK', ${index})">
Mark As Done</button>
Expand Down
5 changes: 2 additions & 3 deletions example01/ArchivedList.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ import html from "../index";
import { connect } from "./store";
import ArchivedTask from "./ArchivedTask";

function ArchiveList(state) {
const { archive } = state;
function ArchiveList({archive}) {
return html`
<h2>Completed Tasks</h2>
<ul>
${archive.map(ArchivedTask)}
${archive.map(task => ArchivedTask({task}))}
</ul>
`;
}
Expand Down
4 changes: 2 additions & 2 deletions example01/ArchivedTask.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import html from "../index";

export default function ArchivedTask(text) {
export default function ArchivedTask({task}) {
return html`
<li style="color:#666; text-decoration:line-through">
${text}
${task}
</li>
`;
}
3 changes: 1 addition & 2 deletions example03/App.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import html from "../index";
import { connect } from "./store";

import TextInput from "./TextInput";
import CharCounter from "./CharCounter";

export default function App(idx) {
return html`
${TextInput(idx)}
${TextInput({idx})}
${CharCounter()}
`;
}
2 changes: 1 addition & 1 deletion example03/TextInput.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import html from "../index";
import { connect } from "./store";

function TextInput({value}, idx) {
function TextInput({value, idx}) {
return html`
<textarea id="text-input-${idx}"
placeholder="Type here…"
Expand Down
5 changes: 3 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ export function createStore(reducer) {
roots.set(root, component);
render();
},
connect(component) {
connect(component, selector = state => state) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a nit / question -- is your preference to keep the selector as the second arg, vs the react-redux style

const ConnectedFoo = connect(state => ({ bar: state.foo.bar }))(Foo);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess (to kinda answer my own question) your way allows connecting to the state without any selectors.

Copy link
Contributor

@bsouthga bsouthga Sep 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which would need to be something like the following in react-redux style,

const ConnectedFoo = connect()(Foo);

// Return a decorated component function.
return (...args) => component(state, ...args);
return (props, ...args) =>
component(Object.assign({}, props, selector(state)), ...args);
},
dispatch(action, ...args) {
state = reducer(state, action, args);
Expand Down
74 changes: 61 additions & 13 deletions test/connect_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,36 +3,38 @@ require = require("@std/esm")(module, {esm: "js"});
const assert = require("assert");
const { createStore } = require("../index")

function counter(state = 0, action) {
switch (action) {
case "INCREMENT":
return state + 1;
default:
return state;
}
}

suite("connect", function() {
let store;
let root;

function counter(state = 0, action) {
switch (action) {
case "INCREMENT":
return state + 1;
default:
return state;
}
}

setup(function() {
store = createStore(counter);
root = document.createElement("div");
});

test("passes the current state as the first argument", function() {
test("pass the current state as the first argument", function() {
const { attach, connect, dispatch } = store;

const TestApp = spyFunction();
const ConnectedTestApp = connect(TestApp);

attach(ConnectedTestApp, root);
dispatch("INCREMENT");
assert.deepEqual(TestApp.args(), [0]);

dispatch("INCREMENT");
assert.deepEqual(TestApp.args(), [1]);
});

test("passes other args after the state", function() {
test("pass other args after the state", function() {
const { attach, connect, dispatch } = store;

const TestComponent = spyFunction();
Expand All @@ -43,8 +45,54 @@ suite("connect", function() {
}

attach(TestApp, root);
dispatch("INCREMENT");
assert.deepEqual(TestComponent.args(), [0, "Foo"]);

dispatch("INCREMENT");
assert.deepEqual(TestComponent.args(), [1, "Foo"]);
});
});

suite("connect with a selector", function() {
let store;
let root;

function counter(state = {current: 0}, action) {
switch (action) {
case "INCREMENT":
return {current: state.current + 1};
default:
return state;
}
}

setup(function() {
store = createStore(counter);
root = document.createElement("div");
});

test("use the identity selector by default", function() {
const { attach, connect, dispatch } = store;

const TestApp = spyFunction();
const ConnectedTestApp = connect(TestApp);

attach(ConnectedTestApp, root);
assert.deepEqual(TestApp.args(), [{current: 0}]);

dispatch("INCREMENT");
assert.deepEqual(TestApp.args(), [{current: 1}]);
});

test("apply the selector to the state", function() {
const { attach, connect, dispatch } = store;

const TestApp = spyFunction();
const ConnectedTestApp = connect(TestApp, state => state.current);

attach(ConnectedTestApp, root);
assert.deepEqual(TestApp.args(), [0]);

dispatch("INCREMENT");
assert.deepEqual(TestApp.args(), [1]);
});
});