From 7ef4aa707d2d53e57273524d16606a360f5f3cdf Mon Sep 17 00:00:00 2001 From: Vladimir Guguiev Date: Sat, 21 Jul 2018 15:18:13 +0200 Subject: [PATCH 1/3] fix flow typing | remove `createSender` API --- __tests__/Context-test.js | 2 +- __tests__/ReComponent-test.js | 6 +- __tests__/ReComponentImmutable-test.js | 2 +- __tests__/RePureComponent-test.js | 2 +- __tests__/UpdateTypes-test.js | 13 +-- src/re.js | 21 ---- type-definitions/ReComponent.js.flow | 18 ++-- type-definitions/__tests__/ReComponent.js | 117 +++++++++++++--------- 8 files changed, 91 insertions(+), 90 deletions(-) diff --git a/__tests__/Context-test.js b/__tests__/Context-test.js index 09a4ca6..1d225c7 100644 --- a/__tests__/Context-test.js +++ b/__tests__/Context-test.js @@ -44,7 +44,7 @@ describe("ReComponent", () => { class Container extends ReComponent { constructor() { super(); - this.handleClick = this.createSender("CLICK"); + this.handleClick = () => this.send({ type: "CLICK" }); this.state = { count: 0 }; } diff --git a/__tests__/ReComponent-test.js b/__tests__/ReComponent-test.js index 9319c8b..bece0d6 100644 --- a/__tests__/ReComponent-test.js +++ b/__tests__/ReComponent-test.js @@ -22,7 +22,7 @@ describe("ReComponent", () => { class Example extends ReComponent { constructor() { super(); - this.handleClick = this.createSender("CLICK"); + this.handleClick = () => this.send({ type: "CLICK" }); this.state = { count: 0 }; } @@ -95,7 +95,7 @@ describe("ReComponent", () => { class Example extends ReComponent { constructor() { super(); - click = this.createSender("CLICK"); + click = () => this.send({ type: "CLICK" }); } static reducer(action, state) { @@ -123,7 +123,7 @@ describe("ReComponent", () => { class ClassPropertyReducer extends ReComponent { constructor() { super(); - click = this.createSender("CLICK"); + click = () => this.send({ type: "CLICK" }); } reducer(action, state) { diff --git a/__tests__/ReComponentImmutable-test.js b/__tests__/ReComponentImmutable-test.js index 44f5709..a61723f 100644 --- a/__tests__/ReComponentImmutable-test.js +++ b/__tests__/ReComponentImmutable-test.js @@ -23,7 +23,7 @@ describe("ReComponentImmutable", () => { class Example extends ReComponent { constructor() { super(); - this.handleClick = this.createSender("CLICK"); + this.handleClick = () => this.send({ type: "CLICK" }); } initialImmutableState(props) { diff --git a/__tests__/RePureComponent-test.js b/__tests__/RePureComponent-test.js index fdfd8f2..5e68717 100644 --- a/__tests__/RePureComponent-test.js +++ b/__tests__/RePureComponent-test.js @@ -21,7 +21,7 @@ describe("RePureComponent", () => { class Example extends RePureComponent { constructor() { super(); - this.handleClick = this.createSender("CLICK"); + this.handleClick = () => this.send({ type: "CLICK" }); this.state = { count: 0 }; } diff --git a/__tests__/UpdateTypes-test.js b/__tests__/UpdateTypes-test.js index b9e0977..2a5b696 100644 --- a/__tests__/UpdateTypes-test.js +++ b/__tests__/UpdateTypes-test.js @@ -31,12 +31,13 @@ describe("UpdateTypes", () => { class ReducerReturns extends ReComponent { constructor() { super(); - noUpdate = this.createSender("NO_UPDATE"); - update = this.createSender("UPDATE"); - sideEffects = this.createSender("SIDE_EFFECTS"); - updateWithSideEffects = this.createSender("UPDATE_WITH_SIDE_EFFECTS"); - invalid = this.createSender("INVALID"); - unhandled = this.createSender("UNHANDLED"); + noUpdate = () => this.send({ type: "NO_UPDATE" }); + update = () => this.send({ type: "UPDATE" }); + sideEffects = () => this.send({ type: "SIDE_EFFECTS" }); + updateWithSideEffects = () => + this.send({ type: "UPDATE_WITH_SIDE_EFFECTS" }); + invalid = () => this.send({ type: "INVALID" }); + unhandled = () => this.send({ type: "UNHANDLED" }); this.state = { count: 0 }; } diff --git a/src/re.js b/src/re.js index 3a9a796..39af873 100644 --- a/src/re.js +++ b/src/re.js @@ -88,9 +88,6 @@ export function Re(Component) { // and return either `NoUpdate()`, `Update(state)`, `SideEffects(fn)`, or // `UpdateWithSideEffects(state, fn)`. // - // To avoid defining functions that call `ReComponent#send` in the render - // method, we also expose a convenience method: `ReComponent#createSender`. - // // @see https://git.io/vh2AY this.send = action => { let sideEffects; @@ -150,24 +147,6 @@ export function Re(Component) { setState.call(this, updater, () => sideEffects && sideEffects(this)); }; - - // Convenience method to create sender functions: Functions that send an - // action to the reducer. The created actions will follow the naming - // conventions of [flux-standard-actions]. - // - // If the sender is called with an argument (like an Event object for an - // event callback), the first argument will be exposed as the `payload`. - // Note that subsequent arguments to a sender are ignored for now. - // - // [flux-standard-actions]: https://github.com/redux-utilities/flux-standard-action - this.createSender = type => { - return payload => { - this.send({ - type, - payload - }); - }; - }; } }; } diff --git a/type-definitions/ReComponent.js.flow b/type-definitions/ReComponent.js.flow index a073442..2a6e0d2 100644 --- a/type-definitions/ReComponent.js.flow +++ b/type-definitions/ReComponent.js.flow @@ -8,8 +8,6 @@ import * as React from "react"; declare opaque type UpdateType; -export type Sender = (a: A) => { action: AT, payload: A }; - declare export function NoUpdate(): {| type: UpdateType |}; declare export function Update(state: S): {| type: UpdateType, state: S |}; declare export function SideEffects( @@ -22,27 +20,25 @@ declare export function UpdateWithSideEffects( declare export class ReComponent< Props, - State = void, - ActionType = string + State, + Action: { +type: string }, > extends React.Component { initialState(props: Props): State; static reducer( - action: { type: ActionType }, + action: Action, state: State ): | {| type: UpdateType |} | {| type: UpdateType, state: $Shape |} - | {| type: UpdateType, sideEffects: ($Subtype>) => mixed |} - | {| type: UpdateType, state: $Shape, sideEffects: ($Subtype>) => mixed |}; - - send(action: { type: ActionType, payload?: mixed }): void; + | {| type: UpdateType, sideEffects: ($Subtype>) => mixed |} + | {| type: UpdateType, state: $Shape, sideEffects: ($Subtype>) => mixed |}; - createSender(actionType: ActionType): Sender; + send(action: Action): void; // This type is only used when initialState returns an Immutable.js data type. // Consider this API unstable. +immutableState: State; } -declare export class RePureComponent extends ReComponent {} +declare export class RePureComponent extends ReComponent {} diff --git a/type-definitions/__tests__/ReComponent.js b/type-definitions/__tests__/ReComponent.js index 1e2bf53..405cccc 100644 --- a/type-definitions/__tests__/ReComponent.js +++ b/type-definitions/__tests__/ReComponent.js @@ -11,35 +11,9 @@ import { UpdateWithSideEffects } from "../../"; -class UntypedActionTypes extends ReComponent<{}, { count: number }> { - handleClick = this.createSender("CLICK"); - // $ExpectError - handleFoo = this.createSender(); - - state = { count: 0 }; +type Action = {| type: "A" |} | {| type: "B" |} | {| type: "C" |} | {| type: "D" |} - static reducer(action, state) { - switch (action.type) { - case "CLICK": - return Update({ count: state.count + 1 }); - default: - return NoUpdate(); - } - } -} -const untypedActionTypes = new UntypedActionTypes(); -untypedActionTypes.send({ type: "CLICK" }); -untypedActionTypes.send({ type: "CLACK" }); -// $ExpectError -untypedActionTypes.send({}); - -untypedActionTypes.handleClick(); -untypedActionTypes.handleClick({}); -untypedActionTypes.handleClick(1); -// $ExpectError -untypedActionTypes.handleClick({}, {}); - -class StateMismatch extends ReComponent<{}, { count: number }> { +class StateMismatch extends ReComponent<{}, { count: number }, Action> { // $ExpectError state = { invalid: "state" }; @@ -50,16 +24,16 @@ class StateMismatch extends ReComponent<{}, { count: number }> { case "B": return Update({ count: 1 }); case "C": - // $ExpectError + // $ExpectError - `count` should be `number` return Update({ count: "1" }); default: - // $ExpectError + // $ExpectError - `invalid` is missing in State return Update({ invalid: "state" }); } } } -class UpdateTypes extends ReComponent<{}, { count: number }> { +class UpdateTypes extends ReComponent<{}, { count: number }, Action> { // Used to test the callback property of SideEffects someClassProperty: number; @@ -72,13 +46,13 @@ class UpdateTypes extends ReComponent<{}, { count: number }> { case "C": return SideEffects((instance: UpdateTypes) => { instance.someClassProperty = 1; - // $ExpectError + // $ExpectError - `instance.someClassProperty` has to be number instance.someClassProperty = "1"; }); default: return UpdateWithSideEffects({ count: 1 }, (instance: UpdateTypes) => { instance.someClassProperty = 1; - // $ExpectError + // $ExpectError - `instance.someClassProperty` has to be number instance.someClassProperty = "1"; }); @@ -86,12 +60,12 @@ class UpdateTypes extends ReComponent<{}, { count: number }> { } } -class TypedActionTypes extends ReComponent<{}, { count: number }, "CLICK"> { - handleClick = this.createSender("CLICK"); - // $ExpectError - handleFoo = this.createSender("CLACK"); - // $ExpectError - handleBar = this.createSender(); +class TypedActionTypes extends ReComponent< + {}, + { count: number }, + {| type: 'CLICK' |} +> { + handleClick = () => this.send({ type: 'CLICK' }); static reducer(action, state) { switch (action.type) { @@ -105,16 +79,16 @@ class TypedActionTypes extends ReComponent<{}, { count: number }, "CLICK"> { const typedActionTypes = new TypedActionTypes(); typedActionTypes.send({ type: "CLICK" }); -// $ExpectError +// $ExpectError - "CLACK" is invalid action type typedActionTypes.send({ type: "CLACK" }); -// $ExpectError +// $ExpectError - invalid action typedActionTypes.send({}); typedActionTypes.handleClick(); +// $ExpectError - `handleClick` expects no arguments typedActionTypes.handleClick({}); +// $ExpectError - `handleClick` expects no arguments typedActionTypes.handleClick(1); -// $ExpectError -typedActionTypes.handleClick({}, {}); // Flow can verify that we've handled every defined action type for us through // what is called [exhaustiveness testing]. @@ -127,27 +101,29 @@ typedActionTypes.handleClick({}, {}); const absurd = (x: empty): T => { throw new Error("absurd"); }; + class ExhaustivelyTypedFailingActionTypes extends ReComponent< {}, { count: number }, - "CLICK" | "CLACK" + {| type: 'CLICK' |} | {| type: 'CLACK' |} > { static reducer(action, state) { switch (action.type) { case "CLICK": return NoUpdate(); default: { - // $ExpectError + // $ExpectError - should be unreachable absurd(action.type); return NoUpdate(); } } } } + class ExhaustivelyTypedPassingActionTypes extends ReComponent< {}, { count: number }, - "CLICK" | "CLACK" + { type: "CLICK" } | { type: "CLACK" } > { static reducer(action, state) { switch (action.type) { @@ -162,3 +138,52 @@ class ExhaustivelyTypedPassingActionTypes extends ReComponent< } } } + + +class FailingPayloadType extends ReComponent< + {}, + { count: number, awesome: boolean }, + { type: "CLICK", payload: number } | { type: "CLACK", payload: boolean } +> { + // $ExpectError - `clicks` should be `number` + handleClick = (clicks: boolean) => this.send({ type: 'CLICK', payload: clicks }); + // $ExpectError - `awesome` should be `boolean` + handleClack = (awesome: number) => this.send({ type: 'CLACK', payload: awesome }); + + static reducer(action, state) { + switch (action.type) { + case "CLICK": + // $ExpectError - `awesome` should be `boolean`, but received `number` + return Update({ awesome: action.payload }); + case "CLACK": + // $ExpectError - `count` should be `number`, but received `boolean` + return Update({ count: action.payload }); + default: { + absurd(action.type); + return NoUpdate(); + } + } + } +} + +class PassingPayloadType extends ReComponent< + {}, + { count: number, awesome: boolean }, + { type: "CLICK", payload: number } | { type: "CLACK", payload: boolean } + > { + handleClick = (clicks: number) => this.send({ type: 'CLICK', payload: clicks }); + handleClack = (awesome: boolean) => this.send({ type: 'CLACK', payload: awesome }); + + static reducer(action, state) { + switch (action.type) { + case "CLICK": + return Update({ count: action.payload }); + case "CLACK": + return Update({ awesome: action.payload }); + default: { + absurd(action.type); + return NoUpdate(); + } + } + } +} From 31a367f5888bb00a6b324c5ee259b70b5bda14eb Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Sat, 21 Jul 2018 17:37:56 +0200 Subject: [PATCH 2/3] Bring back createSender --- src/re.js | 21 ++++++++++++++ type-definitions/ReComponent.js.flow | 1 + type-definitions/__tests__/ReComponent.js | 35 +++++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/src/re.js b/src/re.js index 39af873..3a9a796 100644 --- a/src/re.js +++ b/src/re.js @@ -88,6 +88,9 @@ export function Re(Component) { // and return either `NoUpdate()`, `Update(state)`, `SideEffects(fn)`, or // `UpdateWithSideEffects(state, fn)`. // + // To avoid defining functions that call `ReComponent#send` in the render + // method, we also expose a convenience method: `ReComponent#createSender`. + // // @see https://git.io/vh2AY this.send = action => { let sideEffects; @@ -147,6 +150,24 @@ export function Re(Component) { setState.call(this, updater, () => sideEffects && sideEffects(this)); }; + + // Convenience method to create sender functions: Functions that send an + // action to the reducer. The created actions will follow the naming + // conventions of [flux-standard-actions]. + // + // If the sender is called with an argument (like an Event object for an + // event callback), the first argument will be exposed as the `payload`. + // Note that subsequent arguments to a sender are ignored for now. + // + // [flux-standard-actions]: https://github.com/redux-utilities/flux-standard-action + this.createSender = type => { + return payload => { + this.send({ + type, + payload + }); + }; + }; } }; } diff --git a/type-definitions/ReComponent.js.flow b/type-definitions/ReComponent.js.flow index 2a6e0d2..07264f8 100644 --- a/type-definitions/ReComponent.js.flow +++ b/type-definitions/ReComponent.js.flow @@ -35,6 +35,7 @@ declare export class ReComponent< | {| type: UpdateType, state: $Shape, sideEffects: ($Subtype>) => mixed |}; send(action: Action): void; + createSender(actionType: $ElementType): (mixed) => A; // This type is only used when initialState returns an Immutable.js data type. // Consider this API unstable. diff --git a/type-definitions/__tests__/ReComponent.js b/type-definitions/__tests__/ReComponent.js index 405cccc..f957b16 100644 --- a/type-definitions/__tests__/ReComponent.js +++ b/type-definitions/__tests__/ReComponent.js @@ -187,3 +187,38 @@ class PassingPayloadType extends ReComponent< } } } + +class CreateSenderTest extends ReComponent< + {}, + { count: number }, + {| type: 'CLICK' |} | {| type: 'CLACK', payload: number |} +> { + handleClick = this.createSender("CLICK"); + handleClack = this.createSender("CLACK"); + // $ExpectError - "INVALID" is invalid action type + handleFoo = this.createSender("INVALID"); + // $ExpectError - invalid action type + handleBar = this.createSender(); + + static reducer(action, state) { + return NoUpdate(); + } +} + +const createSenderTest = new CreateSenderTest(); +createSenderTest.send({ type: "CLICK" }); +createSenderTest.send({ type: "CLACK", payload: 0 }); +// $ExpectError - "INVALID" is invalid action type +createSenderTest.send({ type: "INVALID" }); +// $ExpectError - invalid action +createSenderTest.send({}); +// $ExpectError - invalid payload +createSenderTest.send({ type: "CLACK", payload: "CLACK" }); + +// @TODO: Find out how we can assert the payload when using createSender +createSenderTest.handleClick(); +createSenderTest.handleClick({}); +createSenderTest.handleClick(1); +createSenderTest.handleClack(3); +createSenderTest.handleClack(); +createSenderTest.handleClick("sda"); From 467ccd62b2e9229a366afa0c47dfea760b1342fe Mon Sep 17 00:00:00 2001 From: Vladimir Guguiev Date: Sun, 22 Jul 2018 11:31:10 +0200 Subject: [PATCH 3/3] fix review comments --- README.md | 64 ++++++++++++------------------- __tests__/ReComponent-test.js | 8 +--- __tests__/RePureComponent-test.js | 2 +- 3 files changed, 27 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index afb5cb3..6d4cc63 100644 --- a/README.md +++ b/README.md @@ -368,56 +368,33 @@ If you‘re having troubles understanding this example, I recommend the fantasti _[Flow][] is a static type checker for JavaScript. This section is only relevant for you if you‘re using Flow in your application._ -ReComponent comes with first class Flow support built in. By default, a ReComponent will behave like a regular Component and will require props and state to be typed: +ReComponent comes with first class Flow support built in. +When extending `ReComponent`, in addition to the `Props` and `State` types required by regular `React.Component` +we need to specify the third generic parameter which should be a union of all actions used by the component. +This ensures type-safety everywhere in the code of the component where the actions are used and +even allows [exhaustiveness testing] to verify that every action is indeed handled. ```js import * as React from "react"; import { ReComponent, Update } from "react-recomponent"; type Props = {}; -type State = { count: number }; +type State = { count: number, value: string }; +type Action = {| type: "CLICK" |} | {| type: "UPDATE_VALUE", payload: string |}; -class UntypedActionTypes extends ReComponent { - handleClick = this.createSender("CLICK"); - state = { count: 0 }; - - static reducer(action, state) { - switch (action.type) { - case "CLICK": - return Update({ count: state.count + 1 }); - default: - return NoUpdate(); - } - } - - render() { - return ( - - ); - } -} -``` +class TypedActions extends ReComponent { + // NOTE: we use `this.send` API because it ensures type-safety for action's `payload` + handleClick = () => this.send({ type: "CLICK" }); + handleUpdateValue = (newValue: string) => this.send({ type: "UPDATE_VALUE", payload: newValue }); -Without specifying our action types any further, we will allow all `string` values. It is, however, recommended that we type all action types using a union of string literals. This will further tighten the type checks and will even allow [exhaustiveness testing] to verify that every action is indeed handled. - -```js -import * as React from "react"; -import { ReComponent, Update } from "react-recomponent"; - -type Props = {}; -type State = { count: number }; -type ActionTypes = "CLICK"; - -class TypedActionTypes extends ReComponent { - handleClick = this.createSender("CLICK"); state = { count: 0 }; static reducer(action, state) { switch (action.type) { case "CLICK": return Update({ count: state.count + 1 }); + case "UPDATE_VALUE": + return Update({ value: action.payload }); default: { return NoUpdate(); } @@ -426,9 +403,12 @@ class TypedActionTypes extends ReComponent { render() { return ( - + + + + ); } } @@ -438,7 +418,11 @@ Check out the [type definition tests](https://github.com/philipp-spiess/react-re **Known Limitations With Flow:** -- While it is possible to exhaustively type check the reducer, Flow will still require every branch to return an effect. This is why the above examples returns `NoUpdate()` even though the branch can never be reached. +- `this.send` API for sending actions is preferred over `this.createSender`. This is because `this.createSender` + effectively types the payload as `any` (limitation we can't overcome for now), whereas `this.send` provides full type-safety + for actions +- While it is possible to exhaustively type check the reducer, Flow will still require every branch to return an effect. + This is why the above examples returns `NoUpdate()` even though the branch can never be reached. ## API Reference diff --git a/__tests__/ReComponent-test.js b/__tests__/ReComponent-test.js index 131f02d..2802775 100644 --- a/__tests__/ReComponent-test.js +++ b/__tests__/ReComponent-test.js @@ -1,11 +1,7 @@ import React from "react"; import ReactDOM from "react-dom"; -import { - ReComponent, - NoUpdate, - Update, -} from "../src"; +import { ReComponent, NoUpdate, Update } from "../src"; import { click, withConsoleMock } from "./helpers"; @@ -123,7 +119,7 @@ describe("ReComponent", () => { click = () => this.send({ type: "CLICK" }); } - static reducer(action, state) { + reducer(action, state) { return NoUpdate(); } diff --git a/__tests__/RePureComponent-test.js b/__tests__/RePureComponent-test.js index 5e68717..fdfd8f2 100644 --- a/__tests__/RePureComponent-test.js +++ b/__tests__/RePureComponent-test.js @@ -21,7 +21,7 @@ describe("RePureComponent", () => { class Example extends RePureComponent { constructor() { super(); - this.handleClick = () => this.send({ type: "CLICK" }); + this.handleClick = this.createSender("CLICK"); this.state = { count: 0 }; }