gaearon/react-proxy

Proxies React components without unmounting or losing their state

React Proxy

A generic React component proxy used as the new engine by React Hot Loader.

1.x and 2.x

You are looking at the README from the 1.x branch that is widely in use. However we intend to gradually transition projects such as react-transform-hmr to use 2.x that is being developed in master instead. Currently we mirror all releases on both branches.

Requirements

• React 0.13+

Usage

Intended to be used from hot reloading tools like React Hot Loader.
If you’re an application developer, it’s unlikely you’ll want to use it directly.

import React, { Component } from 'react';

class ComponentVersion1 extends Component {
  render() {
    return <div>Before hot update.</div>;
  }
}

class ComponentVersion2 extends Component {
  render() {
    return <div>After hot update.</div>;
  }
}

Without React Proxy:

const rootEl = document.getElementById('root');
React.render(<ComponentVersion1 />, rootEl);

// Will reset state and kill DOM :-(
React.render(<ComponentVersion2 />, rootEl);

With React Proxy:

import { createProxy, getForceUpdate } from 'react-proxy';

// Create a proxy object, given the initial React component class.
const proxy = createProxy(ComponentVersion1);

// Obtain a React class that acts exactly like the initial version.
// This is what we'll use in our app instead of the real component class.
const Proxy = proxy.get();

// Render the component (proxy, really).
React.render(<Proxy />, rootEl);

// Point the proxy to the new React component class by calling update().
// Instances will stay mounted and their state will be intact, but their methods will be updated.
// The update() method returns an array of mounted instances so we can do something with them.
const mountedInstances = proxy.update(ComponentVersion2);

// React Proxy also provides us with getForceUpdate() method that works even if the component
// instance doesn't descend from React.Component, and doesn't have a forceUpdate() method.
const forceUpdate = getForceUpdate(React);

// Force-update all the affected instances!
mountedInstances.forEach(forceUpdate);

Features

• Supports both classic (React.createClass()) and modern (ES6 classes) style
• Supports classes that don’t descend from React.Component
• Supports classes with strict shouldComponentUpdate
• Supports inherited and base classes (although you shouldn’t use inheritance with React)
• Supports classic createClass() autobinding and modern autobind-decorator
• Contains an extensive test suite to avoid regressions
• Preserves displayName
• Preserves enumerability and writability of methods
• Preserves toString() of methods
• Replaces instance getters and setters
• Replaces instance methods preserving their identity
• Replaces bound instance methods preserving their identity
• Because identity is preserved, instance methods already scheduled for setInterval or setTimeout are updated
• Replaces static getters and setters
• Replaces unbound static methods
• Replaces static properties unless they were overwritten by code

Known Limitations

• Does not replace ES7 instance properties
• Does not replace bound static methods
• Replacing a method using autobind-decorator causes its identity to change

Contributing

1. Clone the repository
2. Run npm install
3. Run npm run test:watch
4. Take a look at the existing tests
5. Add tests for the failing case you aim to fix and make them pass
6. Submit a PR!

License

MIT

react-proxy's Issues

Explore using ES2015 Proxies

They might make a ton of things easier.
We can fall back to the old method if they are unavailable.

I don’t have time for this right now but it’s a perfect opportunity to get involved with the project.
The tests should pass :D

If you decide to play with it, please post in this thread so we don’t duplicate the effort.

ES7 property initializers disappear when combined with certain decorator functions

(Note: I encountered this while using react-transform-webpack-hmr, but I believe the issue to be internal to createProxy.)

When using a @decorator function that copies the original component-class into a new one, static properties specified using ES7 initializers are unaccounted for.

For example, here's a slightly modified version of App.js from react-transform-boilerplate:

import React, { Component } from 'react';

// This is TypeScript's implementation of Extend:
var __extends = function (d, b) {
  for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
  function __() { this.constructor = d; }
  __.prototype = b.prototype;
  d.prototype = new __();
};

function decorate (BaseComponent) {
  function DecoratedComponent (...args) {
    console.log("Running decorated constructor!");
    BaseComponent.apply(this, ...args)
  }

  __extends(DecoratedComponent, BaseComponent);

  return DecoratedComponent;
}

@decorate
class Number extends Component {
  static defaultProps = {
    value: 1,
  };

  render() {
    return <h1>{this.props.value} (defaultProps={'' + Number.defaultProps})</h1>;
  }
}

export class App extends Component {
  render() {
    return (
      <div>
        <Number />
        <Number value={42} />
      </div>
    );
  }
}

We expect the rendered output to look like this:

But instead we see this:

For some reason, Counter.defaultProps is not transferred in TypeScript's implementation of __extends when it is defined with ES7 generators.

It may have something to do with how __extends uses hasOwnProperty, but I'm not familiar enough with how react-proxy works under the hood to really take a stab at what might be going on, here.

• If I remove the @decorator, it works as expected.
• If I change @decorator to use ES6's extends, (i.e. class DerivedComponent extends Component ...), it works as expected.
• If I specify defaultProps after the decorator runs (i.e. Counter.defaultProps = ...), it works as expected.

I first encountered this problem when using react-free-style, which is written in TypeScript and provides a decorator function that extends components.

The natural workaround is to avoid static ES7 property initializers, but many of us are already using them, as encouraged by React.

I forked react-transform-boilerplate to make it easier to reproduce the problem:

git clone https://github.com/namuol/react-transform-boilerplate-es7-initializer-bug
cd react-transform-boilerplate-es7-initializer-bug
npm install
npm start
open http://localhost:3000

Issues with actions not being replaced when modules change

I'm using Redux and when I make changes to the actions (action creators file) I can see that the modules are being reloaded but when I invoke the actions the old actions are still being called. I'm having a bit of an issue trying to zero in on the root cause but I'm working on it.

I'm leaving this here for now, this might not be the best place for the issue but I'm quite certain that the issue is not with Redux but something about the constructor not being rerun on update? or, that the closure is capturing the old action reference and won't see the new action references for that reason.

I've tried both with and without mapDispatchToProps same result.

Here's a snippet of the class structure, for reference

class SearchBox extends Component {
  constructor(props) {
    super(props)
    this.state = {
      value: props.value
    }
    this._valueChanged = (e) => {
      this.setState({
        value: e.target.value
      })
    }
    this._valueKeyPressesd = (e) => {
      if (e.charCode === 13) {
        // this.props.dispatch(setTerm({term: this.state.value}))
        this.props.setTerm({term: this.state.value})
      }
    }
    this._searchClicked = (e) => {
      // this.props.dispatch(setTerm({term: this.state.value}))
      this.props.setTerm({term: this.state.value})
    }
  }

  render() {
    return (
      <div className='row'>
        <div className='col-md-8'>
          <input className='form-control' type='text' value={this.state.value} onChange={this._valueChanged} onKeyPress={this._valueKeyPressesd} />
        </div>
        <div className='col-md-4'>
          <button className='btn btn-default' type='button' onClick={this._searchClicked}>Go!</button>
        </div>
      </div>
    )
  }
}

function mapStateToProps(state) {
  return {
    value: state.search.get('term')
  }
}

function mapDispatchToProps(dispatch) {
  return bindActionCreators({ setTerm }, dispatch)
}

Undefined method in __reactAutoBindMap

I have a problem with one of our projects that uses hot reloading with webpackand thus react-proxy.

I get an error "Uncaught TypeError: Cannot read property 'bind' of undefined". The reason this happens is because __reactAutoBindMap has all values undefined after being wrapped in the proxy. The specific class inherits another class which also has a mixin and I suspect that this might be a specific constellation. As far as I can see, there are two probable causes for the issue:

1. createPrototypeProxy has "const currentNames = Object.getOwnPropertyNames(current);", which only picks up own properties and copies them to the proxy.

2. Because of 1., the following snippet then assigns "undefined" to all properties in the original __reactAutoBindMap (from createPrototypeProxy.createAutoBindMap):

   let __reactAutoBindMap = {};
   for (let name in current.__reactAutoBindMap) {
   if (current.__reactAutoBindMap.hasOwnProperty(name)) {
   __reactAutoBindMap[name] = proxy[name];
   }
   }

This happens because proxy now does not have all of the inherited methods.

I can fix the problem by replacing the second fragment with

let __reactAutoBindMap = {};
for (let name in current.__reactAutoBindMap) {
    if (proxy.hasOwnProperty(name)) {
        __reactAutoBindMap[name] = proxy[name];
    }
}

but I am not sure if that does not break something else since I have not completely understood the implications of this code.

Can you shed some light on this please?

Hot reloading via `createClass`

Continued from facebook/react#2797.

The way patching works, the same makeHot function should handle both the original version of the component, and all its updated “versions” which you get from the re-evaluated module code on hot updates. This is how it knows that they all refer to the same class in code, and sets up the patching. So in your case you need to somehow store a map of makeHot functions based on some unique class identifier.

Is this because react-hot-api was derived from webpack and how it sends info about updated modules? Or is it to account for possible renaming of React classes? If the latter, would it make sense to just keep a single reference to the React class and update the methods accordingly for each reload while letting everything else pan out as usual? I still need to dive deeper into both React and react-hot-api's internals, so I'm just making some educated guesses here.

createClass has auto-binding, and new methods after hot reload need to be auto-bound.

Can you elaborate on what exactly the auto-binding is for/what it does? Is that referring to stuff like <div onClick={this.handleClick}> or something else?

ProxyClass prevents filtering on displayName

Hi,
I have a question how do go further in this matter. Essentially we are using react-hot-loder, but I found that in the end it's this library that causes my problem.

Worth mentioning is that this might be an anti pattern but I can't find a better way to do this atm.

In a component we are filtering children based on the displayName to render the results in multiple places.

render() {
  let actions = this.props.children.filter(c => c.type.displayName.indexOf("action") > -1);
  let columns = this.props.children.filter(c => c.type.displayName.indexOf("column") > -1);
  return (<div>
    {actions}
    <table>
    <head>
      <tr>{columns}</tr>
    </thead>
    {...}
  </div>);
}

The problem is that we can't access the displayName due this.props.children contains instances of ProxyClass and where .type.displayName return undefined.

May I ask for som guidance here, is there a better way to solve this or is there a way to access the actual child through the proxy class?

Regards Niklas

Use lodash-es

Currently it depends on lodash.

    lodash "^4.6.1"

This results in 2 copies of lodash being loaded. It would be nicer it it depended only on ES version (lodash-es).
Alternatively, you could add es folder and load it using module option in package.json.

Align with autobinding changes in React master

There were some changes to autobinding internal field names.
We should support the new field names in master: facebook/react#5550

Fix inheritance

Inheritance is bad, but at least we should tests we aren't breaking classes that inherit from something other than Component.

Proxy'd component missing setState in IE 9/10

const React = require('react');
const { createProxy } = require('react-proxy');

const Component = React.createClass({
  getInitialState() {
    return { text: 'hi' };
  },

  componentDidMount() {
    this.setState({ text: 'hey' });
  },

  render() {
    return <span>{this.state.text}</span>;
  },
});

const proxy = createProxy(Component);
const Proxy = proxy.get();

const element = <Proxy />;

React.render(element, document.getElementById('root'));

Only in IE9:

Warning: Something is calling a React component directly. Use a factory or JSX instead. See: https://fb.me/react-legacyfactory 
SCRIPT438: Object doesn't support property or method 'setState'

did not test 10, ok in 11, Edge, Chrome, FF. testing repo: https://github.com/kentor/react-transform-hmr-ie9

Pretty stumped on this one...

I put a debugger on where the Warning would have been emitted and went up the call stack to see what's different from the other browsers. Also here this instanceof Constructor returns false in ie9 vs other browsers.

This is a place of suspicion.

I set the internalInstance to a variable and then went down a stack frame

this which should have been the caller, internalInstance, is not the case in ie9 vs other browsers.

Initialise state when getInitialState() is added after the initial render using HMR

When I'm working with hot reloading and adding state to a React.createClass component which didn't have it before, I usually have to hack in a one-time this.setState(this.getInitialState()) call somewhere to initialise the state and avoid TypeError: this.state is null in render().

Is it possible to detect that a getInitialState() has just been added to a component (or that state is null and getInitialState() exists, or some other heuristic) and have react-proxy do the setState(this.getInitialState()) bit for me, and is react-proxy the right place to do this?

ES7 instance properties

I know that currently react-proxy does not support es7 instance properties. Is there any possibility of its being resolved anytime soon? Also I am learning javascript. So if it is sometime an intermediate can do I would like to try. But wondering where to start.

Uglify borked on 1.1.4

Uglify borked with error message:

> webpack --config webpack.config.prod.js

Hash: df87c9f1484ead5c19e1
Version: webpack 1.12.14
Time: 5804ms
                             Asset    Size  Chunks             Chunk Names
    bundle.df87c9f1484ead5c19e1.js  777 kB       0  [emitted]  main
bundle.df87c9f1484ead5c19e1.js.map  938 kB       0  [emitted]  main
   [0] multi main 28 bytes {0} [built]
    + 269 hidden modules

ERROR in bundle.df87c9f1484ead5c19e1.js from UglifyJs
Unexpected token name «key», expected punc «;» [./~/react-proxy/modules/createClassProxy.js:45,0]

Configuration: https://gist.github.com/airlanggacahya/cc8e1c68e9212221f3ad

Equality check fails

I use the latest react-hot-loader. I have a component that looks roughly like this:

import Child from './Child';

function Parent({ children }) {
  invariant(
    Children.only(children).type === Child,
    'child must be of type Child'
  );
  ...
}

it checks that a child is of type Child. Thing is that the .type is the proxied type, while Child is the original Child, so equality fails.

When I debug and click on .type in Chrome it shows the generated code:

(function(factory,instantiate
/*``*/) {
return function Children() {
         return instantiate(factory, this, arguments);
      }
})

What's the best practice for comparing with a proxy? I could add a static prop and compare it but that seems dirty.

Wrap the render() calls into try-catch

I propose to wrap the render() calls into try-catch because React doesn't know how to recover from those. We can log errors to console and return null or <span style={{ color: 'red' }}>{error.toString()}</span> when that happens.

In some restrictive environments `new Function` throws

We currently use new Function() to generate a function with a matching name. I remember seeing an issue in some other project (can't find it eh) about react-proxy throwing on new Function() in some restrictive environment.

We should probably catch that and, if new Function() throws, use a regular (anonymous) function instead.

Display name should be the wrapped component's name

So that instead of:
Warning: Failed propType: Required prop 'onUpdateEmail' was not specified in 'HotClass'. Check the render method of 'HotClass'.
the developer sees:
Warning: Failed propType: Required prop 'onUpdateEmail' was not specified in 'WrappedComponentName'. Check the render method of 'WrappedComponentName'.

Don't rebind unless necessary

Make sure we don't regress on this:

• gaearon/react-hot-loader#45
• gaearon/react-hot-api@d1bf76a

Components with a strict shouldComponentUpdate() in the middle are not updated

The getForceUpdate(instance) function that ships with React Proxy only updates the component provided to it, and does not update any component below it that returns false from shouldComponentUpdate(). This is especially problematic for React Native where many built-in components like ListView are very optimized, and hot reloading parents seems to have no effects.

Add integration tests

We want to have separate integration tests for libraries that claim to support hot reloading:

• React JSS
• Radium

We also want to test that it works with:

• Omniscient components
• autobind decorator

etc

createClassProxy randomly returns first cached class under Safari JIT

The fix for #52 makes createClassProxy behave oddly when running in the Safari JIT.

From my debugging (difficult since the issue disappears when the code is not JITed -- like when running with the inspector open) the Safari JIT (I'm running Safari 9.0.3 (build 11601.4.4)) makes baseIteratee (called from lodash.find) randomly think that the find predicate ([key]) => key === Component -- is an Object and not a Function.

Because of this the baseMatches function called instead of the ([key]) => key === Component returns the first value in allProxies instead of the desired class or null. This results in components randomly being replaced by other components in react-transform-hmr.

function findProxy(Component) {
  const pair = find(allProxies, ([key]) => key === Component);
  return pair ? pair[1] : null;
}
...
export default function proxyClass(InitialComponent) {
  // Prevent double wrapping.
  // Given a proxy class, return the existing proxy managing it.
  var existingProxy = findProxy(InitialComponent);
  if (existingProxy) {
    return existingProxy;
  }
...
}

function baseIteratee(value) {
  var type = typeof value;
  if (type == 'function') {
    return value;
  }
  if (value == null) {
    return identity;
  }
  if (type == 'object') {
    return isArray(value)
      ? baseMatchesProperty(value[0], value[1])
      : baseMatches(value);
  }
  return property(value);
}

This also occasionally results in a stack-overflow in lodash.toPairs <- lodash.baseMatches <- lodash.baseIteratee <- lodash.find:

RangeError: Maximum call stack size exceeded.
toPairs toPairs.js:25
getMatchData _getMatchData.js:11
baseMatches _baseMatches.js:11
baseIteratee _baseIteratee.js:24
find find.js:41
findProxy createClassProxy.js:58
update createClassProxy.js:137

Helpful error not thrown for stateless components instantiation.

That one is really annoying.
See gaearon/react-hot-loader#488

Cannot preserve state of dynamically generated classes

I just discovered a bit of a gotcha with react-proxy (it might have been somewhere higher in the react-transform stack, this is my best guess as to where the issue belongs).

If you create a component class dynamically, like this contrived example derived from the react-transform-boilerplate project:

var WeirdCounter = (() => {
  var randomNumber = Math.round(Math.random() * 100);
  return class WeirdCounter extends Component {
    // .. irrelevant code copied from Counter elided
    render() {
      return (
        <h1 style={{ color: this.props.color }}>
          {randomNumber} is the best number! ({this.props.increment}): {this.state.counter}
        </h1>
      );
    }
  }
})();

It will render and reload just fine, but its state will be reset each time.

I would have just submitted a PR and added this to "Known Limitations", but I have no idea how to describe it in one concise bullet point. I certainly don't expect this to be a simple fix, or an issue that has any priority whatsoever ;) Just thought that this was an interesting gotcha and wanted to share that with the community.

Could this be used to auto-chunkify bundles?

With https://github.com/webpack/react-proxy-loader you can automatically convert React components to be dynamically loaded via webpack chunks.

However, that code doesn't know the difference between ES6 modules and regular ones.

I'm wondering if this project could be used as the wrapper class in react-proxy-loader to get a more robust loader? Just thinking out loud, the mismatch might be too great. Hoping that someone knowledgeable can confirm either way.

Support pure function components

They should be available in 0.14.

Conflicts with react-mixin

When reloading a component decorated with the react-mixin decorator, it appears that it will attempt to apply the mixin again, throwing a cannot merge returns because both have the "locales" key error, for example, if locales is defined in contextTypes on the mixin.

I suspect this is because react-mixin mutates the class in-place rather than wrapping or copying. I'm not sure there is a sane solution to this, but definitely something to think about.

Consider return proxy from proxy.update method

Hi Dan,

I find it a bit weird, that proxy.update returns list of proxied elements. Probably, it does make sense to return proxy instance instead and expose mountedInstances as an export parameter like this:

import { createProxy, getForceUpdate, mountedInstances } from 'react-proxy';

What do you think about it?

Bind ES6 class methods in constructor

Seems like binding class methods in ES6 class constructor doesn't work with react-transform-hmr (which uses react-proxy)

class Test extends Component({
  constructor(props) {
    super(props);
    this.state = {open: false};
    this.handleMouseDown = this.handleMouseDown.bind(this);
    window.testInstance = this;
  }
  handleMouseDown() {
    console.log(window.testInstance === this); // false
    this.setState({open: true}); // Warning: setState(...): Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op. Please check the code for the  component.
  }
  render {
    return <div onMouseDown={this.handleMouseDown}>Test</div>;
  }
})

Binding in componentWillMount works:

class Test extends Component({
  constructor(props) {
    super(props);
    this.state = {open: false};
  }
  componentWillMount() {
    window.testInstance = this;
    this.handleMouseDown = this.handleMouseDown.bind(this);
  }
  handleMouseDown() {
    console.log(window.testInstance === this); // false
    this.setState({open: true}); // Ok
  }
  render {
    return <div onMouseDown={this.handleMouseDown}>Test</div>;
  }
})

support `createReactClass`

Upgraded React to 15.5 and didn't realize the hmr broke (using react-transform, react-transform-hmr, browserify-hmr)

Why forcing update of the whole tree?

One could call forceUpdate just on the updated mounted instances. This way the amount of calls to render would be minimized and (more importantly) the reference to the root of the tree would become unnecessary.

I tried adding these lines at #createClassProxy.js:176 (branch 2.0.8) and it seems to work fine:

(code adapted from react-deep-force-update)

mountedInstances.forEach((publicInstance) => {
  if (typeof publicInstance.forceUpdate === 'function') {
    publicInstance.forceUpdate();
  } else if (publicInstance.updater && typeof publicInstance.updater.enqueueForceUpdate === 'function') {
    publicInstance.updater.enqueueForceUpdate(publicInstance);
  }
});

and seems to be a tiny bit faster.
Update:
I tested it in a real project: up to 5 times faster, no issues found, no react router warnings anymore.

Don't track mounted instances on server

We might need to move tracking to componentDidMount (and somehow make sure that the shallow renderer invokes it).

README miss spelling

You've written Contibuting instead of Contributing as one of your subheaders in the README.md file

Consider handling removed @autobind decorator

Not sure it's worth pursuing but with current heuristics, removing @autobind midway keeps the method bound. Maybe we could try to handle that for consistency.

Travis + env-specific tests

• Add .travis.yml
• Find a way to run tests both with Babel loose mode and without it

Keep InitialComponent reference in ProxyComponent

According to issue gaearon/react-hot-loader#304, I'd like to propose adding a __original property to the ProxyComponent which keeps a reference to the InitialComponent. So that we can compares element by type:

import MyComponent from './MyComponent';

element.type === MyComponent || element.type.__original === MyComponent;

Consider forcing remount with some heuristics

If constructor, componentWillMount, or componentDidMount code changed, we should probably force remounting. We can use toString() to check (it won't work for classic components though, as their method implementations are inside React itself so that they work with mixins).

3.x fails when the class contains non-configurable properties

We should add a try-catch there or be more careful copying descriptors.

Support facebook/react#4612 if it lands

Support facebook/react#4612 if it lands.

core-js WeakMaps are broken on React Native?

As reported here by @davodesign84 and here, it appears that #45 broke React Native because for some reason WeakMap polyfill from core-js returns undefined:

undefined is not a constructor (evaluating 'new _weakMap2.default()')

Alas, I don’t have React Native at hand and can’t look into why this happens. We just use it like this:

import WeakMap from 'core-js/library/es6/weak-map';

I’ll be grateful if anybody can look into why this fails on RN. Possible alternatives:

• Use a different WeakMap polyfill
• Use a Map polyfill instead of the issue is specific to WeakMap

Fails on IE11 Win10 (no Array.prototype.find)

react-proxy/src/createClassProxy.js

Line 35 in 2d79a5b

const pair = allProxies.find(([key]) => key === Component);

This line is calling [].find which doesn't exist in IE11.

Verify that it doesn't mess up React DevTools

The new ones. :-)

Do you want to use ESLint?

I cloned the repository because I've been meaning to see what's going with class instance properties. (Foolish? Maybe 😐 But in the worst case, I learn something about React internals, Babel plugins, or hot reloading I call that a win 🥇 ) However, when I opened Atom, it wasn't happy about ESLint not being installed. Since it's a weekend and I'm free to yak shave, I'd like to fix that.

I'm opening this issue to ask what approach the maintainers prefer. I can do any combination of the following, ordered according to increasing magnitude of change:

1. Delete the .eslintrc file. All done, problem solved.

2. Add eslint@^0.24.1 and babel-eslint@^4.1.8 to devDependencies.

   Based on the contents of the .eslintrc, I can tell that it was originally used with a v0.x release of ESLint because it overrides some rules that were enabled by default in the v0.x releases - when we shipped v1.0, we disabled all rules by default.

   #54 already addressed the editor complaints in the 1.x branch by adding the missing devDependencies, but it installed ESLint v2, which means quotes is now the only rule enabled in the 1.x branch in this repository.

3. Fix 15 existing lint errors in `src/`.

   $ node_modules/.bin/eslint src
   
   src/createClassProxy.js
      74:8   error  Missing semicolon                                     semi
      81:21  error  The Function constructor is eval                      no-new-func
     119:8   error  existingProxy is already declared in the upper scope  no-shadow
     121:6   error  Expected no return value                              consistent-return
     138:4   error  The '__proto__' property is deprecated                no-proto
     138:31  error  The '__proto__' property is deprecated                no-proto
     226:3   error  Unnecessary semicolon                                 no-extra-semi
     265:9   error  The '__proto__' property is deprecated                no-proto
   
   src/createPrototypeProxy.js
     111:4  error  Expected no return value                consistent-return
     174:4  error  The '__proto__' property is deprecated  no-proto
     190:1  error  Unnecessary semicolon                   no-extra-semi
   
   src/deleteUnknownAutoBindMethods.js
     50:4  error  Expected no return value  consistent-return
     53:4  error  Expected no return value  consistent-return
   
   src/supportsProtoAssignment.js
     4:2  error  The '__proto__' property is deprecated  no-proto
     9:1  error  Unnecessary semicolon                   no-extra-semi
   
   ✖ 15 problems (15 errors, 0 warnings)

4. In `tests/`, enable the `mocha` environment, disable `no-unused-vars`, and fix 19 remaining errors.

   test/consistency.js
     299:6  error  Missing semicolon  semi
   
   test/helpers/createShallowRenderer.js
      12:38  error  Strings must use singlequote  quotes
      13:35  error  Strings must use singlequote  quotes
      14:31  error  Strings must use singlequote  quotes
      15:34  error  Strings must use singlequote  quotes
      16:27  error  Strings must use singlequote  quotes
      17:41  error  Strings must use singlequote  quotes
      18:21  error  Strings must use singlequote  quotes
      25:3   error  "options" is not defined      no-undef
     119:42  error  Missing semicolon             semi
   
   test/instance-method-autobinding.js
     228:4  error  Missing semicolon  semi
   
   test/instance-method.js
     148:1  error  Unnecessary semicolon  no-extra-semi
   
   test/legacy-support.js
     2:54  error  Missing semicolon  semi
   
   test/pure-component.js
     25:3  error  Missing semicolon  semi
   
   test/static-descriptor.js
      98:17  error  Unexpected trailing comma  comma-dangle
     102:17  error  Unexpected trailing comma  comma-dangle
   
   test/static-method.js
      10:5   error  Unnecessary semicolon      no-extra-semi
      22:5   error  Unnecessary semicolon      no-extra-semi
     114:27  error  Unexpected trailing comma  comma-dangle
   
   ✖ 19 problems (19 errors, 0 warnings)

5. Upgrade to a newer version of ESLint, extend eslint:recommended, and fix any lint errors.

   When we stopped enabling default rules in v1.0, we created eslint:recommended, which omits stylistic rules and enables "you probably didn't mean to do this" rules. Extending eslint:recommended would be most similar to the current behavior of using the default rules and disabling a few.

6. Add a lint step to the test script in package.json.

Cannot proxy classes without transpiling them

Currently it throws Class constructors cannot be invoked without 'new'. I forked the project to show the problem. Unfortunately I failed to find a fix. The implementation relies a lot on "classic" prototype inheritance and instantiating objects with CurrentComponent.apply(this, arguments). The best I could get was instantiating a CurrentComponent and copying its properties to this:

 ProxyComponent = function () {
    const current = new CurrentComponent(...arguments);

    Object.getOwnPropertyNames(current).forEach(property => {
      if (RESERVED_STATICS.indexOf(property) > -1) {
        return;
      }
      if (typeof current[property] !== 'function') {
        this[property] = current[property];
      }
    });
  };

Then only 7 tests failed. Still not a fix though.

shouldDeleteModernInstanceMethod shouldn’t call get()

The shouldDeleteModernInstanceMethod assumes that if prototypeDescription.get exists, that get() will return something. When using MobX’s @action, the decorator creates a get that returns undefined, which causes an exception when accessing length on line 31.

I’m not sure if a get() result of undefined should be treated like a length of 0 or get not existing in the first place. (Or if this issue really belongs to MobX.)

createPrototypeProxy.js: proxyMethod() does not copy the length property from the original method

As a result, the length property of the proxiedMethod is always zero.

element.type.displayName differs from the original component (is undefined)

The proxy should have the same displayName like the original component to allow for type checking.

Does not work with React 16.3 forwardRef

Please excuse a somewhat vague report as I’m just investigating a rather involved build system.

Our codebase has (a) a custom component base class, which is generally discouraged; and (b) HOCs that use the new forwardRef() functionality. This lead to a very strange breakage when I tried to migrate some components that currently use our internal BaseComponent to instead extend React.Component (or PureComponent) directly: our React Native app crashed in react-proxy’s update() function with the error 'Expected a constructor.'. It appears that when extending the React classes, the component ends up being not a class, but instead an object with the shape (per Chrome debugger)

{
  $$typeof: Symbol(react.forward_ref),
  render: forwardFn(props, ref) { ... }
}

It’s not clear to me why this does not happen with our custom base class: I suppose forwardRef() treats it differently, being a full-blown ES6 class as opposed to what React provides. It’s also not clear to me exactly where I should be filing this bug: react-proxy, react-transform-hmr, metro, …? Any thoughts welcomed.