Preact: 10.0.0-beta.0 -> 10.0.0-beta.1, react-hot-loader no longer renders the exported component

[resolritter]

reproduction repository: https://github.com/reaysawa/starter-preact-scss

1. clone https://github.com/reaysawa/starter-preact-scss
2. yarn && yarn dev should work with hot-reloading :heavy_check_mark:
3. yarn add preact@next to upgrade from _10.0.0-beta._0 to _10.0.0-beta._1
4. yarn && yarn dev again, but this time it doesn't render :red_circle:

[wardpeet]

I'll try to fix this one as we have the same issue for our preact gatsby plugin I would like to ship

[JoviDeCroock]

I fear this has something to do with _self, what we use now to prevent JSON injection @wardpeet.

Also might be worth checking this out: https://twitter.com/dan_abramov/status/1125846420949434368

[sventschui]

As @JoviDeCroock correctly mentioned: This is caused by the JSON injection prevention. To get things rendered one could patch the patched React.Children.only to set the _ property:

import 'react-hot-loader/patch';

const originalChildrenOnly = React.Children.only;
React.Children.only = function (children) {
  const result = originalChildrenOnly(children);
  result._ = result;
  return result;
};

But then we would need to patch all other patched methods too. This is a no-go IMHO.

I see two ways of fixing this:

• Keep _self set to an object that we share between create-element and diff. From my investigation this would add 11B
• Set _self and __self to the same empty object. This would add 5B

[JoviDeCroock]

Yes, the second solution has come up to me aswell, this would still prevent JSON injection but I'm a bit reluctant about it all. Given that I'm no security expert whatsoever.

[sventschui]

I opened a PR. I'm also not a security expert but it shouldn't differ too much from the original solution as it still bases on object ref equality

[JoviDeCroock]

Yes difference being that I'd assume there's a way to make a refferentially equal object in JSON vs an object that was made to shape the vnode. I'm probably overthinking this, let's go with it and see what others say :D

[developit]

The problem is that the equality is now a comparison to an object that isn't externally accessible. Before, it was possible to create a VNode like this:

const vnode = { type: 'div', props: null, children: 'hello world' };
vnode._ = vnode;

A thought: we had originally proposed to implement VNode checking against __proto__===null. That approach provides the same external simplicity as @sventschui's PR, but avoids locking VNodes to an internal "safe" property reference:

const vnode = { type: 'div', props: null, children: 'hello world', __proto__: null };

// in diff():
if (vnode.__proto__!==null) vnode += '';

[wardpeet]

@developit why did we stray from the proto null approach? (I thought that was the way we added it 😛)

[JoviDeCroock]

Proto null can still be injected on browsers where proto is deprecated.

[wardpeet]

You mean browsers that did not implement it? As I don't think it will ever go away as it will break the web. it won't work on < ie 10 but it's full of security issues anyway.

[sventschui]

The __proto__ approach is also flawed in safari & chrome: JSON.parse('{"__proto__":{"hello":"proto"}}').__proto__ will print {hello: "proto"} in these browsers. Didn't test others

[developit]

@sventschui the check was:

JSON.parse('{"__proto__":null}').constructor === undefined

[sventschui]

Yes I took that away from slack in the meantime, sorry for the misunderstanding. As @JoviDeCroock mentioned, there are other issues with the proto approach 😬