From 23253265ad33c44358163298c342e2843ce56c8c Mon Sep 17 00:00:00 2001 From: Roland Szoke Date: Mon, 9 Mar 2020 13:43:13 +0100 Subject: [PATCH 1/3] fix(displayname): add displayName before React.memo --- .eslintrc.json | 1 + examples/beer-finder/src/App.jsx | 6 +-- examples/beer-finder/src/Beer.jsx | 66 ++++++++++++++++----------- examples/beer-finder/src/BeerList.jsx | 14 +++--- examples/beer-finder/src/NavBar.jsx | 16 ++++--- src/view.js | 6 ++- 6 files changed, 64 insertions(+), 45 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index b7597ab..cc9e7b1 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -14,6 +14,7 @@ "react/prefer-stateless-function": "off", "react/destructuring-assignment": "off", "react/state-in-constructor": "off", + "react/jsx-props-no-spreading": "off", "react/prop-types": "off" }, "globals": { diff --git a/examples/beer-finder/src/App.jsx b/examples/beer-finder/src/App.jsx index a6beb0b..2b9bda4 100644 --- a/examples/beer-finder/src/App.jsx +++ b/examples/beer-finder/src/App.jsx @@ -1,6 +1,6 @@ -import React from "react"; -import NavBar from "./NavBar"; -import BeerList from "./BeerList"; +import React from 'react'; +import NavBar from './NavBar'; +import BeerList from './BeerList'; // if a component does not use any store, it doesn't have to be wrapped with view() // it is safer to wrap everything with view() until you get more comfortable with Easy State diff --git a/examples/beer-finder/src/Beer.jsx b/examples/beer-finder/src/Beer.jsx index 43aa67a..67042e7 100644 --- a/examples/beer-finder/src/Beer.jsx +++ b/examples/beer-finder/src/Beer.jsx @@ -1,30 +1,42 @@ -import React from "react"; -import { view, store } from "react-easy-state"; -import Card from "@material-ui/core/Card"; -import CardMedia from "@material-ui/core/CardMedia"; -import CardContent from "@material-ui/core/CardContent"; +import React from 'react'; +import { view, store } from 'react-easy-state'; +import Card from '@material-ui/core/Card'; +import CardMedia from '@material-ui/core/CardMedia'; +import CardContent from '@material-ui/core/CardContent'; // this is re-rendered whenever the relevant parts of the used data stores change -export default view( - ({ name, description, image_url: imageUrl, food_pairing: foodPairing }) => { - const beer = store({ details: false }); +const Beer = ({ + name, + description, + image_url: imageUrl, + food_pairing: foodPairing, +}) => { + const beer = store({ details: false }); - return ( - (beer.details = !beer.details)} className="beer"> - {!beer.details && } - -

{name}

- {beer.details ? ( -

{description}

- ) : ( -
    - {foodPairing.map(food => ( -
  • {food}
    • - ))} -
- )} - - - ); - } -); + return ( + { + beer.details = !beer.details; + }} + className="beer" + > + {!beer.details && ( + + )} + +

{name}

+ {beer.details ? ( +

{description}

+ ) : ( +
    + {foodPairing.map(food => ( +
  • {food}
    • + ))} +
+ )} + + + ); +}; + +export default view(Beer); diff --git a/examples/beer-finder/src/BeerList.jsx b/examples/beer-finder/src/BeerList.jsx index b542981..ed40330 100644 --- a/examples/beer-finder/src/BeerList.jsx +++ b/examples/beer-finder/src/BeerList.jsx @@ -1,10 +1,10 @@ -import React from "react"; -import { view } from "react-easy-state"; -import appStore from "./appStore"; -import Beer from "./Beer"; +import React from 'react'; +import { view } from 'react-easy-state'; +import appStore from './appStore'; +import Beer from './Beer'; // this is re-rendered whenever the relevant parts of the used data stores change -export default view(() => ( +const BeerList = () => (
{!appStore.beers.length ? (

No matching beers found!

@@ -12,4 +12,6 @@ export default view(() => ( appStore.beers.map(beer => ) )}
-)); +); + +export default view(BeerList); diff --git a/examples/beer-finder/src/NavBar.jsx b/examples/beer-finder/src/NavBar.jsx index 96c2a1f..cfff229 100644 --- a/examples/beer-finder/src/NavBar.jsx +++ b/examples/beer-finder/src/NavBar.jsx @@ -1,11 +1,11 @@ -import React from "react"; -import { view } from "react-easy-state"; -import SearchBar from "material-ui-search-bar"; -import LinearProgress from "@material-ui/core/LinearProgress"; -import appStore from "./appStore"; +import React from 'react'; +import { view } from 'react-easy-state'; +import SearchBar from 'material-ui-search-bar'; +import LinearProgress from '@material-ui/core/LinearProgress'; +import appStore from './appStore'; // this is re-rendered whenever the relevant parts of the used data stores change -export default view(() => ( +const NavBar = () => (
( /> {appStore.isLoading && }
-)); +); + +export default view(NavBar); diff --git a/src/view.js b/src/view.js index 6052773..73ca6ea 100644 --- a/src/view.js +++ b/src/view.js @@ -42,7 +42,7 @@ export function view(Comp) { if (isStatelessComp && hasHooks) { // use a hook based reactive wrapper when we can - ReactiveComp = memo(props => { + ReactiveComp = props => { // use a dummy setState to update the component const [, setState] = useState(); const triggerRender = useCallback(() => setState({}), []); @@ -73,7 +73,9 @@ export function view(Comp) { } finally { isInsideFunctionComponent = false; } - }); + }; + ReactiveComp.displayName = Comp.displayName || Comp.name; + ReactiveComp = memo(ReactiveComp); } else { const BaseComp = isStatelessComp ? Component : Comp; // a HOC which overwrites render, shouldComponentUpdate and componentWillUnmount From 5fb7631e1ad838d089332b8468a095515bd8b0ab Mon Sep 17 00:00:00 2001 From: Roland Szoke Date: Thu, 12 Mar 2020 14:27:20 +0100 Subject: [PATCH 2/3] fix(displayname): refactor memo wrapper on functional components Add Readme for React Dev Tools --- README.md | 26 ++++++++ __tests__/staticProps.test.js | 63 ------------------ __tests__/staticProps.test.jsx | 115 +++++++++++++++++++++++++++++++++ src/view.js | 6 +- 4 files changed, 144 insertions(+), 66 deletions(-) delete mode 100644 __tests__/staticProps.test.js create mode 100644 __tests__/staticProps.test.jsx diff --git a/README.md b/README.md index 8e4f6df..3820fe9 100644 --- a/README.md +++ b/README.md @@ -407,6 +407,32 @@ This is not necessary if you use React Router 4.4+. You can find more details an

+
+Usage with React Developer Tools. +

+ +If you want React Developer Tools to recognize your reactive view components with names, you have to pass a **named component** to the `view` wrapper function instead of an anonymous one. + +```jsx +import React from 'react'; +import { view, store } from 'react-easy-state'; +import Table from 'rc-table'; +import cloneDeep from 'lodash/cloneDeep'; + +const user = store({ + name: 'Rick', +}); + +const componentName = () => ( +
{user.name}
+); + +export default view(componentName); +``` + +
+

+
Passing nested data to third party components.

diff --git a/__tests__/staticProps.test.js b/__tests__/staticProps.test.js deleted file mode 100644 index ddd9151..0000000 --- a/__tests__/staticProps.test.js +++ /dev/null @@ -1,63 +0,0 @@ -/* eslint-disable react/forbid-foreign-prop-types */ -/* eslint-disable no-multi-assign */ -import { Component } from 'react'; -// eslint-disable-next-line import/no-unresolved -import { view } from 'react-easy-state'; - -describe('static props', () => { - test('view() should proxy static properties from wrapped components', () => { - class Comp extends Component {} - function FuncComp() {} - - Comp.displayName = FuncComp.displayName = 'Name'; - Comp.contextTypes = FuncComp.contextTypes = {}; - Comp.propTypes = FuncComp.propTypes = {}; - Comp.defaultProps = FuncComp.defaultProps = {}; - Comp.customProp = FuncComp.customProp = {}; - - const ViewComp = view(Comp); - const ViewFuncComp = view(FuncComp); - - expect(ViewComp.displayName).toBe(Comp.displayName); - expect(ViewComp.contextTypes).toBe(Comp.contextTypes); - expect(ViewComp.propTypes).toBe(Comp.propTypes); - expect(ViewComp.defaultProps).toBe(Comp.defaultProps); - expect(ViewComp.customProp).toBe(Comp.customProp); - - expect(ViewFuncComp.displayName).toBe(FuncComp.displayName); - expect(ViewFuncComp.contextTypes).toBe(FuncComp.contextTypes); - expect(ViewFuncComp.propTypes).toBe(FuncComp.propTypes); - expect(ViewFuncComp.defaultProps).toBe(FuncComp.defaultProps); - expect(ViewFuncComp.customProp).toBe(FuncComp.customProp); - }); - - test('view() should proxy static methods', () => { - class Comp extends Component { - static getDerivedStateFromError() {} - - static customMethod() {} - } - - const ViewComp = view(Comp); - expect(ViewComp.getDerivedStateFromError).toBe( - Comp.getDerivedStateFromError, - ); - expect(ViewComp.customMethod).toBe(Comp.customMethod); - }); - - test('view() should proxy static getters', () => { - class Comp extends Component { - static get defaultProp() { - return { key: 'value' }; - } - - static get customProp() { - return { key: 'hello' }; - } - } - - const ViewComp = view(Comp); - expect(ViewComp.defaultProps).toEqual(Comp.defaultProps); - expect(ViewComp.customProp).toEqual(Comp.customProp); - }); -}); diff --git a/__tests__/staticProps.test.jsx b/__tests__/staticProps.test.jsx new file mode 100644 index 0000000..2902854 --- /dev/null +++ b/__tests__/staticProps.test.jsx @@ -0,0 +1,115 @@ +/* eslint-disable react/forbid-foreign-prop-types */ +/* eslint-disable no-multi-assign */ +import React, { Component } from 'react'; +import { render, cleanup } from '@testing-library/react/pure'; +// eslint-disable-next-line import/no-unresolved +import { view } from 'react-easy-state'; +import PropTypes from 'prop-types'; + +describe('static props', () => { + afterEach(cleanup); + + test('view() should proxy defaultProps for class components', () => { + class MyCustomCompName extends Component { + render() { + return
{this.props.name}
; + } + } + + MyCustomCompName.defaultProps = { + name: 'Bob', + }; + + const WrappedComp = view(MyCustomCompName); + const { container } = render(); + expect(container).toHaveTextContent('Bob'); + }); + + test('view() should proxy defaultProps for functional components', () => { + const MyCustomCompName = props => { + return
{props.name}
; + }; + + MyCustomCompName.defaultProps = { + name: 'Bob', + }; + + const WrappedComp = view(MyCustomCompName); + const { container } = render(); + expect(container).toHaveTextContent('Bob'); + }); + + test('view() should proxy propTypes for class components', () => { + class MyCustomCompName extends Component { + render() { + return
{this.props.name}
; + } + } + + MyCustomCompName.propTypes = { + name: PropTypes.string.isRequired, + }; + + const ViewComp = view(MyCustomCompName); + + const errorSpy = jest + .spyOn(console, 'error') + .mockImplementation(message => + expect(message.indexOf('Failed prop type')).not.toBe(-1), + ); + render(); + expect(errorSpy).toHaveBeenCalled(); + errorSpy.mockRestore(); + }); + + test('view() should proxy propTypes for functional components', () => { + const MyCustomCompName = props => { + return
{props.number}
; + }; + + MyCustomCompName.propTypes = { + number: PropTypes.number.isRequired, + }; + + const ViewComp = view(MyCustomCompName); + + const errorSpy = jest + .spyOn(console, 'error') + .mockImplementation(message => + expect(message.indexOf('Failed prop type')).not.toBe(-1), + ); + render(); + expect(errorSpy).toHaveBeenCalled(); + errorSpy.mockRestore(); + }); + + test('view() should proxy static methods', () => { + class Comp extends Component { + static getDerivedStateFromError() {} + + static customMethod() {} + } + + const ViewComp = view(Comp); + expect(ViewComp.getDerivedStateFromError).toBe( + Comp.getDerivedStateFromError, + ); + expect(ViewComp.customMethod).toBe(Comp.customMethod); + }); + + test('view() should proxy static getters', () => { + class Comp extends Component { + static get defaultProp() { + return { key: 'value' }; + } + + static get customProp() { + return { key: 'hello' }; + } + } + + const ViewComp = view(Comp); + expect(ViewComp.defaultProps).toEqual(Comp.defaultProps); + expect(ViewComp.customProp).toEqual(Comp.customProp); + }); +}); diff --git a/src/view.js b/src/view.js index 73ca6ea..fa919b3 100644 --- a/src/view.js +++ b/src/view.js @@ -74,8 +74,6 @@ export function view(Comp) { isInsideFunctionComponent = false; } }; - ReactiveComp.displayName = Comp.displayName || Comp.name; - ReactiveComp = memo(ReactiveComp); } else { const BaseComp = isStatelessComp ? Component : Comp; // a HOC which overwrites render, shouldComponentUpdate and componentWillUnmount @@ -170,5 +168,7 @@ export function view(Comp) { }); } - return ReactiveComp; + return isStatelessComp && hasHooks + ? memo(ReactiveComp) + : ReactiveComp; } From 030222b4be8039d5ea0325606f12a27113328bef Mon Sep 17 00:00:00 2001 From: Roland Szoke Date: Fri, 13 Mar 2020 15:26:31 +0100 Subject: [PATCH 3/3] docs(readme): update react dev tools description --- README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index 3820fe9..ec7dbaa 100644 --- a/README.md +++ b/README.md @@ -411,13 +411,11 @@ This is not necessary if you use React Router 4.4+. You can find more details an Usage with React Developer Tools.

-If you want React Developer Tools to recognize your reactive view components with names, you have to pass a **named component** to the `view` wrapper function instead of an anonymous one. +If you want React Developer Tools to recognize your reactive view components' names, you have to pass either a **named function** or an anonymous function with **name inference** to the `view` wrapper. ```jsx import React from 'react'; import { view, store } from 'react-easy-state'; -import Table from 'rc-table'; -import cloneDeep from 'lodash/cloneDeep'; const user = store({ name: 'Rick',