diff --git a/CHANGELOG.md b/CHANGELOG.md index aa6d61e3..b4fc8394 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * The minimum support version of React is 16.8.0 * Killed the possibility to directly pass store names to `observer`. Always use `inject` instead. (This was deprecate for a long time already). `observer(["a", "b"], component)` should now be written as `inject("a", "b")(component)`. * `observer` components no longer automatically recover from errors (to prevent potential memory leaks). Instead, this is the responsibility of error boundaries. +* `inject` now supports ref forwarding. As such, the `.wrappedInstance` property has been removed since refs can be used instead. **Improvements** diff --git a/src/index.d.ts b/src/index.d.ts index 41ab6570..fae93079 100644 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -27,7 +27,6 @@ export type IStoresToProps< export type IWrappedComponent

= { wrappedComponent: IReactComponent

- wrappedInstance: React.ReactInstance | undefined } // Ideally we would want to return React.ComponentClass>, diff --git a/src/inject.js b/src/inject.js index 34d9d4ed..d1f70842 100644 --- a/src/inject.js +++ b/src/inject.js @@ -1,4 +1,4 @@ -import { Component, createElement } from "react" +import React, { Component, createElement } from "react" import hoistStatics from "hoist-non-react-statics" import * as PropTypes from "./propTypes" import { observer } from "./observer" @@ -33,7 +33,7 @@ const proxiedInjectorProps = { /** * Store Injection */ -function createStoreInjector(grabStoresFn, component, injectNames) { +function createStoreInjector(grabStoresFn, component, injectNames, makeReactive) { let displayName = "inject-" + (component.displayName || @@ -43,42 +43,36 @@ function createStoreInjector(grabStoresFn, component, injectNames) { if (injectNames) displayName += "-with-" + injectNames class Injector extends Component { - static displayName = displayName - - storeRef = instance => { - this.wrappedInstance = instance - } - render() { // Optimization: it might be more efficient to apply the mapper function *outside* the render method // (if the mapper is a function), that could avoid expensive(?) re-rendering of the injector component // See this test: 'using a custom injector is not too reactive' in inject.js - let newProps = {} - for (let key in this.props) - if (this.props.hasOwnProperty(key)) { - newProps[key] = this.props[key] - } - var additionalProps = - grabStoresFn(this.context.mobxStores || {}, newProps, this.context) || {} - for (let key in additionalProps) { - newProps[key] = additionalProps[key] - } + const { forwardRef, ...props } = this.props - if (!isStateless(component)) { - newProps.ref = this.storeRef + Object.assign( + props, + grabStoresFn(this.context.mobxStores || {}, props, this.context) || {} + ) + + if (forwardRef && !isStateless(component)) { + props.ref = this.props.forwardRef } - return createElement(component, newProps) + return createElement(component, props) } } - - // Static fields from component should be visible on the generated Injector - hoistStatics(Injector, component) - - Injector.wrappedComponent = component + if (makeReactive) Injector = observer(Injector) Object.defineProperties(Injector, proxiedInjectorProps) - return Injector + // Support forward refs + const InjectHocRef = React.forwardRef((props, ref) => + React.createElement(Injector, { ...props, forwardRef: ref }) + ) + // Static fields from component should be visible on the generated Injector + hoistStatics(InjectHocRef, component) + InjectHocRef.wrappedComponent = component + InjectHocRef.displayName = displayName + return InjectHocRef } function grabStoresByName(storeNames) { @@ -106,25 +100,19 @@ function grabStoresByName(storeNames) { * or a function that manually maps the available stores from the context to props: * storesToProps(mobxStores, props, context) => newProps */ -export default function inject(/* fn(stores, nextProps) or ...storeNames */) { +export default function inject(/* fn(stores, nextProps) or ...storeNames */ ...storeNames) { let grabStoresFn if (typeof arguments[0] === "function") { grabStoresFn = arguments[0] - return function(componentClass) { - let injected = createStoreInjector(grabStoresFn, componentClass) - injected.isMobxInjector = false // supress warning - // mark the Injector as observer, to make it react to expressions in `grabStoresFn`, - // see #111 - injected = observer(injected) - injected.isMobxInjector = true // restore warning - return injected - } + return componentClass => + createStoreInjector(grabStoresFn, componentClass, grabStoresFn.name, true) } else { - const storeNames = [] - for (let i = 0; i < arguments.length; i++) storeNames[i] = arguments[i] - grabStoresFn = grabStoresByName(storeNames) - return function(componentClass) { - return createStoreInjector(grabStoresFn, componentClass, storeNames.join("-")) - } + return componentClass => + createStoreInjector( + grabStoresByName(storeNames), + componentClass, + storeNames.join("-"), + false + ) } } diff --git a/test/inject.test.js b/test/inject.test.js index 2ed20e98..8f68918e 100644 --- a/test/inject.test.js +++ b/test/inject.test.js @@ -7,6 +7,7 @@ import * as mobx from "mobx" import { action, observable, computed } from "mobx" import { observer, inject, Provider } from "../src" import { createTestRoot, sleepHelper, withConsole } from "./index" +import renderer from "react-test-renderer" const testRoot = createTestRoot() @@ -14,7 +15,7 @@ describe("inject based context", () => { test("basic context", () => { const C = inject("foo")( observer( - createClass({ + class X extends React.Component { render() { return (

@@ -23,7 +24,7 @@ describe("inject based context", () => {
) } - }) + } ) ) const B = () => @@ -254,7 +255,34 @@ describe("inject based context", () => { expect(wrapper.find("div").text()).toBe("context:bar84") }) - test("support static hoisting, wrappedComponent and wrappedInstance", async () => { + test("inject forwards ref", async () => { + class FancyComp extends React.Component { + render() { + this.didRender = true + return null + } + + doSomething() {} + } + + const ref = React.createRef() + const component = renderer.create() + expect(typeof ref.current.doSomething).toBe("function") + expect(ref.current.didRender).toBe(true) + + const InjectedFancyComp = inject("bla")(FancyComp) + const ref2 = React.createRef() + + const component2 = renderer.create( + + + + ) + expect(typeof ref2.current.doSomething).toBe("function") + expect(ref2.current.didRender).toBe(true) + }) + + test("support static hoisting, wrappedComponent and ref forwarding", async () => { class B extends React.Component { render() { this.testField = 1 @@ -273,16 +301,18 @@ describe("inject based context", () => { expect(C.bla2 === B.bla2).toBeTruthy() expect(Object.keys(C.wrappedComponent.propTypes)).toEqual(["x"]) - const wrapper = mount() - await sleepHelper(10) - expect(wrapper.instance().wrappedInstance.testField).toBe(1) + const ref = React.createRef() + + const wrapper = renderer.create() + expect(ref.current.testField).toBe(1) }) - test("warning is printed when attaching contextTypes to HOC", () => { + test.skip("warning is printed when attaching contextTypes to HOC", () => { + // TODO: can be removed once using modern context? const msg = [] const baseWarn = console.warn console.warn = m => msg.push(m) - const C = inject(["foo"])( + const C = inject("foo")( createClass({ displayName: "C", render() { @@ -324,6 +354,7 @@ describe("inject based context", () => { displayName: "C", render() { expect(this.props.y).toEqual(3) + expect(this.props.x).toBeUndefined() return null } @@ -398,7 +429,7 @@ describe("inject based context", () => { console.warn = baseWarn }) - test("using a custom injector is reactive", () => { + test("using a custom injector is reactive", async () => { const user = mobx.observable({ name: "Noa" }) const mapper = stores => ({ name: stores.user.name }) const DisplayName = props =>

{props.name}

diff --git a/test/observer.test.js b/test/observer.test.js index 52f30d56..74ed29e7 100644 --- a/test/observer.test.js +++ b/test/observer.test.js @@ -276,30 +276,6 @@ test("changing state in render should fail", () => { mobx._resetGlobalState() }) -test("component should not be inject", () => { - const msg = [] - const baseWarn = console.warn - console.warn = m => msg.push(m) - - observer( - inject("foo")( - createClass({ - render() { - return ( -
- context: - {this.props.foo} -
- ) - } - }) - ) - ) - - expect(msg.length).toBe(1) - console.warn = baseWarn -}) - test("observer component can be injected", () => { const msg = [] const baseWarn = console.warn