From 22e48e3be00490801bb3c1d89fdc2bc9fa9c4a7f Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Tue, 11 Jun 2019 09:32:28 +0200 Subject: [PATCH] [REF] store: simplify connect API Closes #158 --- doc/store.md | 6 +- src/store.ts | 190 ++++++++++++++++++------------------ tests/store.test.ts | 154 ++++++++++++++++++----------- tools/playground/samples.js | 4 +- 4 files changed, 199 insertions(+), 155 deletions(-) diff --git a/doc/store.md b/doc/store.md index a4f58676..bcfd49d7 100644 --- a/doc/store.md +++ b/doc/store.md @@ -176,7 +176,7 @@ function mapStoreToProps(state) { value: state.counter }; } -const ConnectedCounter = owl.connect(mapStoreToProps)(Counter); +const ConnectedCounter = owl.connect(Counter, mapStoreToProps); const counter = new ConnectedCounter({ store, qweb }); ``` @@ -199,5 +199,5 @@ The arguments of `connect` are: - `deep`: [only useful if no hashFunction is given] if false, only watch for top level state changes (true by default) -The `connect` function returns a function that takes a `Component` as argument -and returns a sub Class of this `Component` that is connected to the `store` +The `connect` function returns a sub class of the given `Component` which is +connected to the `store`. diff --git a/src/store.ts b/src/store.ts index 57cacbef..b062c1ad 100644 --- a/src/store.ts +++ b/src/store.ts @@ -150,10 +150,20 @@ type Constructor = new (...args: any[]) => T; interface EnvWithStore extends Env { store: Store; } +type HashFunction = (a: any, b: any) => number; +interface StoreOptions { + getStore?(Env): Store; + hashFunction?: HashFunction; + deep?: boolean; +} let nextID = 1; -export function connect(mapStateToProps, options: any = {}) { +export function connect( + Comp: Constructor>, + mapStoreToProps, + options: StoreOptions = {} +) { let hashFunction = options.hashFunction || null; const getStore = options.getStore || (env => env.store); @@ -181,103 +191,95 @@ export function connect(mapStateToProps, options: any = {}) { }; } - return function( - Comp: Constructor> - ) { - const Result = class extends Comp { - constructor(parent, props?: any) { - const env = parent instanceof Component ? parent.env : parent; - const store = getStore(env); - const ownProps = Object.assign({}, props || {}); - const storeProps = mapStateToProps( - store.state, - ownProps, - store.getters - ); - const mergedProps = Object.assign({}, props || {}, storeProps); - super(parent, mergedProps); - (this.__owl__).ownProps = ownProps; - (this.__owl__).currentStoreProps = storeProps; - (this.__owl__).store = store; - (this.__owl__).storeHash = hashFunction( - { - state: store.state, - storeProps: storeProps, - revNumber, - deepRevNumber - }, - { - currentStoreProps: storeProps - } - ); - } - /** - * We do not use the mounted hook here for a subtle reason: we want the - * updates to be called for the parents before the children. However, - * if we use the mounted hook, this will be done in the reverse order. - */ - _callMounted() { - (this.__owl__).store.on("update", this, this._checkUpdate); - super._callMounted(); - } - willUnmount() { - (this.__owl__).store.off("update", this); - super.willUnmount(); - } + const Result = class extends Comp { + constructor(parent, props?: any) { + const env = parent instanceof Component ? parent.env : parent; + const store = getStore(env); + const ownProps = Object.assign({}, props || {}); + const storeProps = mapStoreToProps(store.state, ownProps, store.getters); + const mergedProps = Object.assign({}, props || {}, storeProps); + super(parent, mergedProps); + (this.__owl__).ownProps = ownProps; + (this.__owl__).currentStoreProps = storeProps; + (this.__owl__).store = store; + (this.__owl__).storeHash = (hashFunction)( + { + state: store.state, + storeProps: storeProps, + revNumber, + deepRevNumber + }, + { + currentStoreProps: storeProps + } + ); + } + /** + * We do not use the mounted hook here for a subtle reason: we want the + * updates to be called for the parents before the children. However, + * if we use the mounted hook, this will be done in the reverse order. + */ + _callMounted() { + (this.__owl__).store.on("update", this, this._checkUpdate); + super._callMounted(); + } + willUnmount() { + (this.__owl__).store.off("update", this); + super.willUnmount(); + } - _checkUpdate() { - const ownProps = (this.__owl__).ownProps; - const storeProps = mapStateToProps( + _checkUpdate() { + const ownProps = (this.__owl__).ownProps; + const storeProps = mapStoreToProps( + (this.__owl__).store.state, + ownProps, + (this.__owl__).store.getters + ); + const options: any = { + currentStoreProps: (this.__owl__).currentStoreProps + }; + const storeHash = (hashFunction)( + { + state: (this.__owl__).store.state, + storeProps: storeProps, + revNumber, + deepRevNumber + }, + options + ); + let didChange = options.didChange; + if (storeHash !== (this.__owl__).storeHash) { + didChange = true; + (this.__owl__).storeHash = storeHash; + } + if (didChange) { + (this.__owl__).currentStoreProps = storeProps; + this._updateProps(ownProps, false); + } + } + _updateProps(nextProps, forceUpdate, patchQueue?: any[]) { + if ((this.__owl__).ownProps !== nextProps) { + (this.__owl__).currentStoreProps = mapStoreToProps( (this.__owl__).store.state, - ownProps, + nextProps, (this.__owl__).store.getters ); - const options: any = { - currentStoreProps: (this.__owl__).currentStoreProps - }; - const storeHash = hashFunction( - { - state: (this.__owl__).store.state, - storeProps: storeProps, - revNumber, - deepRevNumber - }, - options - ); - let didChange = options.didChange; - if (storeHash !== (this.__owl__).storeHash) { - didChange = true; - (this.__owl__).storeHash = storeHash; - } - if (didChange) { - (this.__owl__).currentStoreProps = storeProps; - this._updateProps(ownProps, false); - } } - _updateProps(nextProps, forceUpdate, patchQueue?: any[]) { - if ((this.__owl__).ownProps !== nextProps) { - (this.__owl__).currentStoreProps = mapStateToProps( - (this.__owl__).store.state, - nextProps, - (this.__owl__).store.getters - ); - } - (this.__owl__).ownProps = nextProps; - const mergedProps = Object.assign( - {}, - nextProps, - (this.__owl__).currentStoreProps - ); - return super._updateProps(mergedProps, forceUpdate, patchQueue); - } - }; - - // we assign here a unique name to the resulting anonymous class. - // this is necessary for Owl to be able to properly deduce templates. - // Otherwise, all connected components would have the same name, and then - // each component after the first will necessarily have the same template. - let name = `ConnectedComponent${nextID++}`; - Object.defineProperty(Result, "name", { value: name }); - return Result; + (this.__owl__).ownProps = nextProps; + const mergedProps = Object.assign( + {}, + nextProps, + (this.__owl__).currentStoreProps + ); + return super._updateProps(mergedProps, forceUpdate, patchQueue); + } }; + + // we assign here a unique name to the resulting anonymous class. + // this is necessary for Owl to be able to properly deduce templates. + // Otherwise, all connected components would have the same name, and then + // each component after the first will necessarily have the same template. + let name = `ConnectedComponent${nextID++}`; + Object.defineProperty(Result, "name", { value: name }); + return Result; } diff --git a/tests/store.test.ts b/tests/store.test.ts index 05cecf02..7353a408 100644 --- a/tests/store.test.ts +++ b/tests/store.test.ts @@ -411,10 +411,13 @@ describe("connecting a component to store", () => { state.todos.push({ msg }); } }; - function mapStateToProps(s) { + function mapStoreToProps(s) { return { todos: s.todos }; } - const TodoApp = connect(mapStateToProps)(App); + const TodoApp = connect( + App, + mapStoreToProps + ); const store = new Store({ state, mutations }); (env).store = store; const app = new TodoApp(env); @@ -434,7 +437,7 @@ describe("connecting a component to store", () => { state.todos[0].title = title; } }; - function mapStateToProps(s) { + function mapStoreToProps(s) { return { todos: s.todos }; } const store = new Store({ state, mutations }); @@ -451,13 +454,15 @@ describe("connecting a component to store", () => { class App extends Component {} const DeepTodoApp = connect( - mapStateToProps, + App, + mapStoreToProps, { deep: true } - )(App); + ); const ShallowTodoApp = connect( - mapStateToProps, + App, + mapStoreToProps, { deep: false } - )(App); + ); (env).store = store; const deepTodoApp = new DeepTodoApp(env); const shallowTodoApp = new ShallowTodoApp(env); @@ -501,15 +506,16 @@ describe("connecting a component to store", () => { } } }); - function mapStateToProps(s) { + function mapStoreToProps(s) { return { todos: s.todos }; } const TodoApp = connect( - mapStateToProps, + App, + mapStoreToProps, { getStore: () => store } - )(App); + ); const app = new TodoApp(env); await app.mount(fixture); @@ -532,7 +538,10 @@ describe("connecting a component to store", () => { } } - const ConnectedChild = connect(s => s)(Child); + const ConnectedChild = connect( + Child, + s => s + ); env.qweb.addTemplate( "Parent", @@ -574,10 +583,13 @@ describe("connecting a component to store", () => { env.qweb.addTemplate("TodoItem", ``); class TodoItem extends Component {} - const ConnectedTodo = connect((state, props) => { - const todo = state.todos.find(t => t.id === props.id); - return todo; - })(TodoItem); + const ConnectedTodo = connect( + TodoItem, + (state, props) => { + const todo = state.todos.find(t => t.id === props.id); + return todo; + } + ); env.qweb.addTemplate( "TodoList", @@ -591,10 +603,13 @@ describe("connecting a component to store", () => { widgets = { ConnectedTodo }; } - function mapStateToProps(state) { + function mapStoreToProps(state) { return { todos: state.todos }; } - const ConnectedTodoList = connect(mapStateToProps)(TodoList); + const ConnectedTodoList = connect( + TodoList, + mapStoreToProps + ); (env).store = store; const app = new ConnectedTodoList(env); @@ -632,13 +647,16 @@ describe("connecting a component to store", () => { ` ); class TodoItem extends Component {} - const ConnectedTodo = connect((state, props, getters) => { - const todo = state.todos.find(t => t.id === props.id); - return { - activeTodoText: getters.text(todo.id), - importantTodoText: getters.importantTodoText() - }; - })(TodoItem); + const ConnectedTodo = connect( + TodoItem, + (state, props, getters) => { + const todo = state.todos.find(t => t.id === props.id); + return { + activeTodoText: getters.text(todo.id), + importantTodoText: getters.importantTodoText() + }; + } + ); env.qweb.addTemplate( "TodoList", @@ -652,10 +670,13 @@ describe("connecting a component to store", () => { widgets = { ConnectedTodo }; } - function mapStateToProps(state) { + function mapStoreToProps(state) { return { todos: state.todos }; } - const ConnectedTodoList = connect(mapStateToProps)(TodoList); + const ConnectedTodoList = connect( + TodoList, + mapStoreToProps + ); (env).store = store; const app = new ConnectedTodoList(env); @@ -669,9 +690,12 @@ describe("connecting a component to store", () => { test("connected component is updated when props are updated", async () => { env.qweb.addTemplate("Beer", ``); class Beer extends Component {} - const ConnectedBeer = connect((state, props) => { - return state.beers[props.id]; - })(Beer); + const ConnectedBeer = connect( + Beer, + (state, props) => { + return state.beers[props.id]; + } + ); env.qweb.addTemplate( "App", @@ -717,10 +741,13 @@ describe("connecting a component to store", () => { const store = new Store({ state, mutations }); (env).store = store; - function mapStateToProps(state) { + function mapStoreToProps(state) { return { beers: state.beers, otherKey: 1 }; } - const ConnectedApp = connect(mapStateToProps)(App); + const ConnectedApp = connect( + App, + mapStoreToProps + ); const app = new ConnectedApp(env); await app.mount(fixture); @@ -743,13 +770,16 @@ describe("connecting a component to store", () => { ` ); class Beer extends Component {} - const ConnectedBeer = connect((state, props) => { - return { - selected: state.beers[props.id], - consumed: state.beers[state.consumedID] || null, - taster: state.taster - }; - })(Beer); + const ConnectedBeer = connect( + Beer, + (state, props) => { + return { + selected: state.beers[props.id], + consumed: state.beers[state.consumedID] || null, + taster: state.taster + }; + } + ); env.qweb.addTemplate( "App", @@ -812,13 +842,16 @@ describe("connecting a component to store", () => { ` ); class Beer extends Component {} - const ConnectedBeer = connect((state, props) => { - return { - selected: state.beers[props.id], - consumed: state.beers[state.consumedID] || null, - taster: state.taster - }; - })(Beer); + const ConnectedBeer = connect( + Beer, + (state, props) => { + return { + selected: state.beers[props.id], + consumed: state.beers[state.consumedID] || null, + taster: state.taster + }; + } + ); env.qweb.addTemplate( "App", @@ -909,18 +942,24 @@ describe("connecting a component to store", () => { class Parent extends Component { widgets = { Child: ConnectedChild }; } - const ConnectedParent = connect(function(s) { - steps.push("parent"); - return { current: s.current, isvisible: s.isvisible }; - })(Parent); + const ConnectedParent = connect( + Parent, + function(s) { + steps.push("parent"); + return { current: s.current, isvisible: s.isvisible }; + } + ); env.qweb.addTemplate("Child", ``); class Child extends Component {} - const ConnectedChild = connect(function(s, props) { - steps.push("child"); - return { msg: s.msg[props.key] }; - })(Child); + const ConnectedChild = connect( + Child, + function(s, props) { + steps.push("child"); + return { msg: s.msg[props.key] }; + } + ); const state = { current: "a", msg: { a: "a", b: "b" } }; const mutations = { @@ -953,9 +992,12 @@ describe("connecting a component to store", () => { steps.push("patched"); } } - const ConnectedApp = connect(function(s) { - return { msg: s.msg }; - })(App); + const ConnectedApp = connect( + App, + function(s) { + return { msg: s.msg }; + } + ); const state = { msg: "a" }; const mutations = { diff --git a/tools/playground/samples.js b/tools/playground/samples.js index 649b2e03..4ebe6f3e 100644 --- a/tools/playground/samples.js +++ b/tools/playground/samples.js @@ -432,7 +432,7 @@ class TodoItem extends owl.Component { //------------------------------------------------------------------------------ // TodoApp //------------------------------------------------------------------------------ -function mapStateToProps(state) { +function mapStoreToProps(state) { return { todos: state.todos }; @@ -489,7 +489,7 @@ class TodoApp extends owl.Component { } } -const ConnectedTodoApp = owl.connect(mapStateToProps)(TodoApp); +const ConnectedTodoApp = owl.connect(TodoApp, mapStoreToProps); //------------------------------------------------------------------------------ // App Initialization