From 9a92db4028c991be558f2d408226eb34d34ee252 Mon Sep 17 00:00:00 2001 From: mai <168496714@qq.com> Date: Tue, 30 Jan 2018 14:04:36 +0800 Subject: [PATCH 1/9] Add Observer render and inject sugar #387 --- src/observer.js | 33 ++++++++++++++++++-- test/observer.test.js | 71 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 92 insertions(+), 12 deletions(-) diff --git a/src/observer.js b/src/observer.js index ac44c7b9..d4fe7582 100644 --- a/src/observer.js +++ b/src/observer.js @@ -1,5 +1,5 @@ import { Atom, Reaction, extras } from "mobx" -import { Component } from "react" +import React, { Component } from "react" import { findDOMNode as baseFindDOMNode } from "react-dom" import EventEmitter from "./utils/EventEmitter" import inject from "./inject" @@ -349,12 +349,41 @@ function mixinLifecycleEvents(target) { } // TODO: support injection somehow as well? -export const Observer = observer(({ children }) => children()) +export const Observer = observer(({ children, inject: observerInject, render }) => { + const component = children || render + if (typeof component === "undefined") { + return null + } + if (!observerInject) { + return component() + } + const InjectComponent = inject(observerInject)(component) + return +}) Observer.displayName = "Observer" Observer.propTypes = { + render: (propValue, key, componentName, location, propFullName) => { + if (typeof propValue["children"] === "function") { + return + } + if (typeof propValue[key] !== "function") + return new Error( + "Invalid prop `" + + propFullName + + "` of type `" + + typeof propValue[key] + + "` supplied to" + + " `" + + componentName + + "`, expected `function`." + ) + }, children: (propValue, key, componentName, location, propFullName) => { + if (typeof propValue["render"] === "function") { + return + } if (typeof propValue[key] !== "function") return new Error( "Invalid prop `" + diff --git a/test/observer.test.js b/test/observer.test.js index 3b47b733..52431c1f 100644 --- a/test/observer.test.js +++ b/test/observer.test.js @@ -4,7 +4,7 @@ import ReactDOM from "react-dom" import ReactDOMServer from "react-dom/server" import TestUtils from "react-dom/test-utils" import * as mobx from "mobx" -import { observer, inject, onError, offError, useStaticRendering, Observer } from "../" +import { observer, inject, onError, offError, useStaticRendering, Observer, Provider } from "../" import { createTestRoot, sleepHelper, asyncReactDOMRender } from "./index" import ErrorCatcher from "./ErrorCatcher" @@ -16,12 +16,6 @@ const testRoot = createTestRoot() const getDNode = (obj, prop) => obj.$mobx.values[prop] -const asyncRender = (element, root) => { - return new Promise(resolve => { - ReactDOM.render() - }) -} - /* use TestUtils.renderIntoDocument will re-mounted the component with with different props some misunderstanding will be cause? @@ -209,7 +203,6 @@ describe("does not views alive when using static rendering", () => { }) test("no re-rendering on static rendering", () => { - expect(renderCount).toBe(1) data.z = "hello" expect(renderCount).toBe(1) expect(TestUtils.findRenderedDOMComponentWithTag(element, "div").innerHTML).toBe("hi") @@ -533,9 +526,10 @@ describe("it rerenders correctly if some props are non-observables - 2", () => { } mobx.reaction(() => odata.x, v => console.log(v)) - - beforeAll(async () => { + + beforeAll(async done => { await asyncReactDOMRender(, testRoot) + done() }) test("init renderCount === 1", () => { @@ -757,3 +751,60 @@ test.skip("195 - should throw if trying to overwrite lifecycle methods", () => { /Cannot assign to read only property 'componentWillMount'/ ) }) + +describe("use Observer inject and render sugar should work ", () => { + test("use render without inject should be correct", async () => { + const Comp = () => ( +
+ {123}} /> +
+ ) + await asyncReactDOMRender(, testRoot) + expect(testRoot.querySelector("span").innerHTML).toBe("123") + }) + + test("use children without inject should be correct", async () => { + const Comp = () => ( +
+ {props => {123}} +
+ ) + await asyncReactDOMRender(, testRoot) + expect(testRoot.querySelector("span").innerHTML).toBe("123") + }) + + test("use render with inject should be correct", async () => { + const Comp = () => ( +
+ ({ h: store.h, w: store.w })} + render={props => {`${props.h} ${props.w}`}} + /> +
+ ) + const A = () => ( + + + + ) + await asyncReactDOMRender(, testRoot) + expect(testRoot.querySelector("span").innerHTML).toBe("hello world") + }) + + test("use children with inject should be correct", async () => { + const Comp = () => ( +
+ ({ h: store.h, w: store.w })}> + {props => {`${props.h} ${props.w}`}} + +
+ ) + const A = () => ( + + + + ) + await asyncReactDOMRender(
, testRoot) + expect(testRoot.querySelector("span").innerHTML).toBe("hello world") + }) +}) From 1edd04add35ba40db4fd6315b6763e25f6b7033d Mon Sep 17 00:00:00 2001 From: mai <168496714@qq.com> Date: Tue, 30 Jan 2018 14:05:33 +0800 Subject: [PATCH 2/9] Update observer test --- test/observer.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/observer.test.js b/test/observer.test.js index 52431c1f..ecde7ed4 100644 --- a/test/observer.test.js +++ b/test/observer.test.js @@ -5,7 +5,7 @@ import ReactDOMServer from "react-dom/server" import TestUtils from "react-dom/test-utils" import * as mobx from "mobx" import { observer, inject, onError, offError, useStaticRendering, Observer, Provider } from "../" -import { createTestRoot, sleepHelper, asyncReactDOMRender } from "./index" +import { createTestRoot, sleepHelper, asyncReactDOMRender, asyncRender } from "./" import ErrorCatcher from "./ErrorCatcher" /** From 1c541006e11b26d6e756d9dc03b206879f9f0da8 Mon Sep 17 00:00:00 2001 From: mai <168496714@qq.com> Date: Wed, 31 Jan 2018 12:35:07 +0800 Subject: [PATCH 3/9] Add typescript definitions --- src/index.d.ts | 2 +- test/ts/compile-ts.tsx | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/index.d.ts b/src/index.d.ts index b687cc3e..69ff1f16 100644 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -65,7 +65,7 @@ export function onError(cb: (error: Error) => void): () => void export class Provider extends React.Component {} -export class Observer extends React.Component<{ children?: () => React.ReactNode }, {}> {} +export class Observer extends React.Component<{ children?: () => React.ReactNode, render?: () => React.ReactNode, inject?: IStoresToProps | string[] }, {}> {} export function useStaticRendering(value: boolean): void diff --git a/test/ts/compile-ts.tsx b/test/ts/compile-ts.tsx index 83994866..4c1aa0ce 100644 --- a/test/ts/compile-ts.tsx +++ b/test/ts/compile-ts.tsx @@ -191,6 +191,18 @@ class ObserverTest extends Component { } } +class ObserverTest2 extends Component { + render() { + return
test
} />; + } +} + +class ObserverTest3 extends Component { + render() { + return
test
} />; + } +} + @observer class ComponentWithoutPropsAndState extends Component<{}, {}> { componentDidUpdate() { From 13b416dd5ea1ca0d08cecc014f5b81e6ab7517af Mon Sep 17 00:00:00 2001 From: mai <168496714@qq.com> Date: Thu, 1 Feb 2018 21:19:01 +0800 Subject: [PATCH 4/9] Add Observer render and inject sugar usage in README --- README.md | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8bf134d3..d702e4fa 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ It takes as children a single, argumentless function which should return exactly The rendering in the function will be tracked and automatically re-rendered when needed. This can come in handy when needing to pass render function to external components (for example the React Native listview), or if you dislike the `observer` decorator / function. - +And it can work with `Provider` use inject and render property / or just takes as children, detail in Example2.(PS:using children has a priority than render , so dont use at the same time) Example: ```javascript @@ -105,6 +105,38 @@ React.render(, document.body) person.name = "Mike" // will cause the Observer region to re-render ``` +Example2: + +```javascript +class App extends React.Component { + render() { + return ( + + + + ) + } +} +const Comp = () => ( +
+ ({ h: store.h, w: store.w })} + render={props => {`${props.h} ${props.w}`}} + /> +
) +/* or +const Comp = () => ( +
+ ({ h: store.h, w: store.w })}> + {props => {`${props.h} ${props.w}`}} + +
) +*/ + +React.render(, document.body) +// will get the same result showing hello world in span tag +``` + ### Global error handler with `onError` If a component throws an error, this logs to the console but does not 'crash' the app, so it might go unnoticed. @@ -438,3 +470,5 @@ Data will have one of the following formats: WeakMap. Its `get` function returns the associated reactive component of the given node. The node needs to be precisely the root node of the component. This map is only available after invoking `trackComponents`. + + From 827a05447c9f3aec951b90b62d75b5f8226ea62f Mon Sep 17 00:00:00 2001 From: mai <168496714@qq.com> Date: Thu, 1 Feb 2018 22:44:42 +0800 Subject: [PATCH 5/9] Update README --- README.md | 57 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index d702e4fa..3f3da28a 100644 --- a/README.md +++ b/README.md @@ -82,8 +82,6 @@ It takes as children a single, argumentless function which should return exactly The rendering in the function will be tracked and automatically re-rendered when needed. This can come in handy when needing to pass render function to external components (for example the React Native listview), or if you dislike the `observer` decorator / function. -And it can work with `Provider` use inject and render property / or just takes as children, detail in Example2.(PS:using children has a priority than render , so dont use at the same time) -Example: ```javascript class App extends React.Component { @@ -105,38 +103,63 @@ React.render(, document.body) person.name = "Mike" // will cause the Observer region to re-render ``` -Example2: +In case you are a fan of render props, you can use that instead of children. Be advised, that you cannot use both approaches at once, children have a precedence. + +Here goes example showing render prop only ```javascript -class App extends React.Component { - render() { - return ( - - - - ) - } -} + const Comp = () => (
({ h: store.h, w: store.w })} - render={props => {`${props.h} ${props.w}`}} + render={() => hello world} />
) /* or const Comp = () => (
- ({ h: store.h, w: store.w })}> - {props => {`${props.h} ${props.w}`}} + + {() => hello world}
) */ -React.render(, document.body) +React.render(, document.body) // will get the same result showing hello world in span tag ``` +Observer can also inject the stores simply by passing a selector function. + +Example with inject + +```javascript + +const NameDisplayer = ({ name }) =>

{name}

+ +const user = mobx.observable({ + name: "Noa" +}) + +const UserNameDisplayer = ()=>( + ({user:stores.user})} + render={props => ()} + /> +) +/* or + ()} + /> +*/ + +const App = () => ( + + + +) +``` + ### Global error handler with `onError` If a component throws an error, this logs to the console but does not 'crash' the app, so it might go unnoticed. From fac7e873b80c9f5d3ccd8f4284461a6c6799f183 Mon Sep 17 00:00:00 2001 From: mai <168496714@qq.com> Date: Thu, 1 Feb 2018 22:57:47 +0800 Subject: [PATCH 6/9] Update example --- README.md | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 3f3da28a..c64d2dfa 100644 --- a/README.md +++ b/README.md @@ -108,24 +108,23 @@ In case you are a fan of render props, you can use that instead of children. Be Here goes example showing render prop only ```javascript +class App extends React.Component { + render() { + return ( +
+ {this.props.person.name} +
{props.person.name}
} + /> +
+ ) + } +} + +const person = observable({ name: "John" }) -const Comp = () => ( -
- hello world} - /> -
) -/* or -const Comp = () => ( -
- - {() => hello world} - -
) -*/ - -React.render(, document.body) -// will get the same result showing hello world in span tag +React.render(, document.body) +person.name = "Mike" // will cause the Observer region to re-render ``` Observer can also inject the stores simply by passing a selector function. @@ -146,12 +145,6 @@ const UserNameDisplayer = ()=>( render={props => ()} /> ) -/* or - ()} - /> -*/ const App = () => ( From f8c2cfbfbc705220d9c955c13fd8c191b48067c9 Mon Sep 17 00:00:00 2001 From: mai <168496714@qq.com> Date: Fri, 2 Feb 2018 09:11:57 +0800 Subject: [PATCH 7/9] Remove bogus lines --- README.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/README.md b/README.md index c64d2dfa..828aa69f 100644 --- a/README.md +++ b/README.md @@ -104,8 +104,7 @@ person.name = "Mike" // will cause the Observer region to re-render ``` In case you are a fan of render props, you can use that instead of children. Be advised, that you cannot use both approaches at once, children have a precedence. - -Here goes example showing render prop only +Example ```javascript class App extends React.Component { @@ -126,9 +125,7 @@ const person = observable({ name: "John" }) React.render(, document.body) person.name = "Mike" // will cause the Observer region to re-render ``` - Observer can also inject the stores simply by passing a selector function. - Example with inject ```javascript From 484aec20ef6d8c151fa6eadfa67010c975d70525 Mon Sep 17 00:00:00 2001 From: mai <168496714@qq.com> Date: Fri, 2 Feb 2018 18:08:20 +0800 Subject: [PATCH 8/9] Update Observer Props types Check --- src/observer.js | 55 +++++++++++++++++++++---------------------------- 1 file changed, 23 insertions(+), 32 deletions(-) diff --git a/src/observer.js b/src/observer.js index d4fe7582..de1517bc 100644 --- a/src/observer.js +++ b/src/observer.js @@ -363,37 +363,28 @@ export const Observer = observer(({ children, inject: observerInject, render }) Observer.displayName = "Observer" -Observer.propTypes = { - render: (propValue, key, componentName, location, propFullName) => { - if (typeof propValue["children"] === "function") { - return - } - if (typeof propValue[key] !== "function") - return new Error( - "Invalid prop `" + - propFullName + - "` of type `" + - typeof propValue[key] + - "` supplied to" + - " `" + - componentName + - "`, expected `function`." - ) - }, - children: (propValue, key, componentName, location, propFullName) => { - if (typeof propValue["render"] === "function") { - return - } - if (typeof propValue[key] !== "function") - return new Error( - "Invalid prop `" + - propFullName + - "` of type `" + - typeof propValue[key] + - "` supplied to" + - " `" + - componentName + - "`, expected `function`." - ) +const ObserverPropsCheck = (props,key,componentName,location,propFullName)=>{ + const extraKey = key === "children" ? "render" : "children" + if(typeof propValue[key] === "function" && typeof propValue[extraKey] === "function" ){ + return new Error("Invalid prop,do not use children and render in the same time in`" + componentName ) + } + + if (typeof propValue[key] === "function" || typeof propValue[extraKey] === "function") { + return } + return new Error( + "Invalid prop `" + + propFullName + + "` of type `" + + typeof propValue[key] + + "` supplied to" + + " `" + + componentName + + "`, expected `function`." + ) +} + +Observer.propTypes = { + render: ObserverPropsCheck, + children: ObserverPropsCheck, } From d4596daa1dc3715403ce87e7236644d49a40b806 Mon Sep 17 00:00:00 2001 From: mai <168496714@qq.com> Date: Sun, 4 Feb 2018 10:57:10 +0800 Subject: [PATCH 9/9] Fix Observer example --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 828aa69f..408c01b8 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,7 @@ class App extends React.Component {
{this.props.person.name}
{props.person.name}
} + render= {() =>
{this.props.person.name}
} />
)