Proxy'd component missing setState in IE 9/10

[kentor]

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.

[gaearon]

Woah woah, that's the best issue description I've seen in a while. 👍

I think it happens because IE did not support assigning __proto__ before IE11.
We rely on __proto__ assignment to set up the prototype chain to point to the new version of the class.

I'm not sure if there's any meaningful fallback we could use..

[kentor]

Yep, that lack of __proto__ assignment would do it and it doesn't look like there are any workarounds for IE < 11.

[glenjamin]

If you can detect it, you can fall back to prop copying.

[okonet]

I can confirm this is also happening in IE10. Any workaround for this?

[gaearon]

The biggest issue for me is that I don't have IE at hand, and can't verify whether any changes to the code fix this. If you'd like to work on this please look here:

react-proxy/src/createClassProxy.js

Lines 88 to 89 in d1b975d

	// Set up the same prototype for inherited statics
	ProxyClass.__proto__ = NextClass.__proto__;

This is the line which, from what I understand, has no effect in IE < 11 because it doesn't support __proto__. If there is a way to detect this at runtime and manually copy descriptors defined on base class (React.Component and potentially other base classes) right onto the ProxyClass, that would probably solve the issue. But of course you'd need IE to actually verify whether the problem is fixed.

[nhunzaker]

Just hit this. Testing on IE10 using the VMWare image via https://dev.modern.ie/tools/vms/mac/. I can't dig into it at the moment, but I thought I'd plug the site in the interim.

[SimenB]

Babel docs contains a caveat about __proto__ in IE < 11, maybe using the transform helps? https://babeljs.io/docs/advanced/caveats/#classes-10-and-below-

[seantimm]

I didn't have any luck getting the transform to work. Anyone else?

[SimenB]

I've also noticed that the transform in the docs is now a plugin, you can try that instead of optional?

https://www.npmjs.com/package/babel-plugin-proto-to-assign

[kentor]

The plugin does transform that line from proxy.__proto__ = next to _defaults(proxy, next);, but it seems like what that does is a shallow copy of the properties from next to proxy. In other words setState is still undefined because it is higher up in the prototype chain (on next.__proto__), so using that transform won't work.

[gaearon]

Never mind the transform.
It's extremely basic and won't help us.

I don't know how to fix this.
I tried fiddling more with prototypes but I'm afraid I'm out of my depth here.

I'd say we likely won't support IE 9 and IE 10 as dev environment anymore.

[nhunzaker]

Dropping <= IE10 support is fine by me as long as it's done in a clean way. I would love for browsers that don't have access to __proto__ to not receive hot-loading treatment and display a warning in the console instead if at all possible.

Right now I just export an environment variable in my build to disable hot loading when testing in IE10 (HOT=FALSE npm start). This works fine, but I'd love to not have to worry about it.

[kentor]

I agree with @nhunzaker. I'm fine with that as well, since I didn't expect the hot reloading stuff to work in IE9 anyway.

[gaearon]

Yeah let's do that. Anybody would like to make a PR?

We should have a feature test (supportsProtoAssignment.js) and if it fails, we should console.error() a warning, return the class as is from get(), and ignore update() calls.

We should also find a way to mock the feature test for unit tests.

[nhunzaker]

I probably can't get to it until Thursday, but I'd be happy to dig in.

[gaearon]

I added you to collabs, feel free to work in a branch. Don't commit to master. ;-)

[SimenB]

Only supporting hot reloading in newer browsers is perfectly fine for me as well!
Would it be possible to set a target for ie8 and not ie9? 😀

[gaearon]

Basically we'll just give you your component back if __proto__ feature test fails so it should work on IE8 too.

[nhunzaker]

Just an update here. I was hoping to achieve at least IE9 support by playing around with prototypes (maybe this stirs some inspiration):

var prototype = Object.create(React.Component)
var proxy = Object.create(prototype)

// Instead of re-assigning __proto__, operate on prototype

However I get into trouble mapping the properties from the component over to prototype. It's a fascinating problem, but I'm definitely out of my league here.

I'm wrapping up work on testing __proto__ support right now, something like:

export default function supportsProtoAssignment () {
  return Object.prototype === ({}).__proto__
};

Now I just need to figure out the best way to test this...

@gaearon We can obviously do manual testing, but you have an ideas how we can automate it? I imagine a whole load of tests will fail if we run the whole suite in < IE11. Right now, the only thing that I can think of is to stub out supportsProtoAssignment and confirm it doesn't apply the proxy treatment.

[gaearon]

Right now, the only thing that I can think of is to stub out supportsProtoAssignment and confirm it doesn't apply the proxy treatment.

👍

[nhunzaker]

Huzzah! Any follow up work I can do?
On Dec 28, 2015 1:37 PM, "Dan Abramov" notifications@github.com wrote:

Closed #27 #27 via #34
#34.

—
Reply to this email directly or view it on GitHub
#27 (comment).

[gaearon]

@nhunzaker

Two things:

1. I'm getting a warning running tests. I don't think the feature test works correctly because {} is not really expected to have __proto__ defined—object literals don't have prototype.
2. I'm not sure what the feature test is testing. It checks whether __proto__ is an object on the argument but it claims to check whether it can be reassigned (supportsProtoAssignment). Does IE have no __proto__ at all, or does it only fail on reassigning? Also, why do we need an argument at all? Why not check it like this:

let x = {};
let y = { supports: true };
try {
  x.__proto__ = y;
} catch (err) {}

export default function supportsProtoAssignment() {
  return x.supports || false;
};

(Not tested but should match the intended behavior from the name.)

I'm not sure how we'd test this behavior in the unit tests though.

[gaearon]

I think I probably solved this with d34f121.

[gaearon]

Note that React Proxy 2.x isn't yet used in React Transform HMR so it'll take some time to get this available directly.

[gaearon]

I backported it to 1.x. Should be out both in 2.0.1 and 1.1.2.

[nhunzaker]

Awesome! Sorry for the extra leg work.
On Dec 28, 2015 2:47 PM, "Dan Abramov" notifications@github.com wrote:

I backported it to 1.x. Should be out both in 2.0.1 and 1.1.2.

—
Reply to this email directly or view it on GitHub
#27 (comment).

[gaearon]

No problem, thanks for contributing!