Skip to content
This repository has been archived by the owner on Dec 31, 2020. It is now read-only.

Add Observer render and inject sugar #387 #403

Merged
merged 9 commits into from
Feb 4, 2018
Merged
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
51 changes: 49 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,6 @@ The rendering in the function will be tracked and automatically re-rendered when
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.

Example:

```javascript
class App extends React.Component {
render() {
Expand All @@ -105,6 +103,53 @@ React.render(<App person={person} />, document.body)
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.
Example

```javascript
class App extends React.Component {
render() {
return (
<div>
{this.props.person.name}
<Observer
render= {() => <div>{this.props.person.name}</div>}
/>
</div>
)
}
}

const person = observable({ name: "John" })

React.render(<App person={person} />, 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

const NameDisplayer = ({ name }) => <h1>{name}</h1>

const user = mobx.observable({
name: "Noa"
})

const UserNameDisplayer = ()=>(
<Observer
inject={(stores)=>({user:stores.user})}
render={props => (<NameDisplayer name={props.user.name}/>)}
/>
)

const App = () => (
<Provider userStore={user}>
<UserNameDisplayer />
</Provider>
)
```

### 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.
Expand Down Expand Up @@ -438,3 +483,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`.


2 changes: 1 addition & 1 deletion src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export function onError(cb: (error: Error) => void): () => void

export class Provider extends React.Component<any, {}> {}

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

Expand Down
50 changes: 35 additions & 15 deletions src/observer.js
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -349,22 +349,42 @@ 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
Copy link
Contributor

Choose a reason for hiding this comment

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

I am thinking if there shouldn't be some warning if both props are used. Perhaps that's what you had in mind with that propTypes check? Except you need to return message describing a problem. Returning undefined means it's valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ennn , thinging, first i prefer show warning by propTypes if both props
and coding like this

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`."
    )
}

'>< but seem complexity
or do same props check in Observer Component instead in propType

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea, checking with prop types like this is probably enough. I don't think it adds complexity, it's a friendly behavior toward user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great !

if (typeof component === "undefined") {
return null
}
if (!observerInject) {
return component()
}
const InjectComponent = inject(observerInject)(component)
return <InjectComponent />
})

Observer.displayName = "Observer"

Observer.propTypes = {
children: (propValue, key, componentName, location, propFullName) => {
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,
}
73 changes: 62 additions & 11 deletions test/observer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ 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 { createTestRoot, sleepHelper, asyncReactDOMRender } from "./index"
import { observer, inject, onError, offError, useStaticRendering, Observer, Provider } from "../"
import { createTestRoot, sleepHelper, asyncReactDOMRender, asyncRender } from "./"
import ErrorCatcher from "./ErrorCatcher"

/**
Expand All @@ -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(<element />)
})
}

/*
use TestUtils.renderIntoDocument will re-mounted the component with with different props
some misunderstanding will be cause?
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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(<Parent odata={odata} />, testRoot)
done()
})

test("init renderCount === 1", () => {
Expand Down Expand Up @@ -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 = () => (
<div>
<Observer render={props => <span>{123}</span>} />
</div>
)
await asyncReactDOMRender(<Comp />, testRoot)
expect(testRoot.querySelector("span").innerHTML).toBe("123")
})

test("use children without inject should be correct", async () => {
const Comp = () => (
<div>
<Observer>{props => <span>{123}</span>}</Observer>
</div>
)
await asyncReactDOMRender(<Comp />, testRoot)
expect(testRoot.querySelector("span").innerHTML).toBe("123")
})

test("use render with inject should be correct", async () => {
const Comp = () => (
<div>
<Observer
inject={store => ({ h: store.h, w: store.w })}
render={props => <span>{`${props.h} ${props.w}`}</span>}
/>
</div>
)
const A = () => (
<Provider h="hello" w="world">
<Comp />
</Provider>
)
await asyncReactDOMRender(<A />, testRoot)
expect(testRoot.querySelector("span").innerHTML).toBe("hello world")
})

test("use children with inject should be correct", async () => {
const Comp = () => (
<div>
<Observer inject={store => ({ h: store.h, w: store.w })}>
{props => <span>{`${props.h} ${props.w}`}</span>}
</Observer>
</div>
)
const A = () => (
<Provider h="hello" w="world">
<Comp />
</Provider>
)
await asyncReactDOMRender(<A />, testRoot)
expect(testRoot.querySelector("span").innerHTML).toBe("hello world")
})
})
12 changes: 12 additions & 0 deletions test/ts/compile-ts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,18 @@ class ObserverTest extends Component<any, any> {
}
}

class ObserverTest2 extends Component<any, any> {
render() {
return <Observer render={() => <div>test</div>} />;
}
}

class ObserverTest3 extends Component<any, any> {
render() {
return <Observer inject={["store"]} render={() => <div>test</div>} />;
}
}

@observer
class ComponentWithoutPropsAndState extends Component<{}, {}> {
componentDidUpdate() {
Expand Down