"stateful" react injection

[timse]

I guess this is a very constructed case, but I am wondering about https://github.com/webcomponents/react-integration/blob/master/src/index.js#L30 .

What was the decision for:

opts = assign(defaults, opts);

intead of

opts = assign({}, defaults, opts);

Again one should imagine that if you inject React and ReactDOM once, it will be injected all the time, however in a very constructed use case we could have some code like this:

const SomeComponent = reactify(SomeWebComponent);

const SomeOtherComponent = reactify(SomeOtherWebComponent);

if I were to be mean I could now call reactifiy before all those calls like so:

try{
    reactify(null, {React: null, ReactDom: null})
} catch (e){}

killing all following calls to reactify as they will no longer have access to React or ReactDOM.

In fact if you remove the , { ReactDOM: null } part from this line: https://github.com/webcomponents/react-integration/blob/master/test/unit/errors.js#L11

the test will still be green, as React is still null. Setting ReactDOM to null does not cause the throw here.

[treshugart]

This seems to be to just be an oversight.

[timse]

aye :) 👍

[treshugart]

Thanks, @timse!

[timse]

you are welcome,

one more thought i has was, that now looking at it im wondering if the object-assign polyfill is needed. (Probably a very immature optimization given that probably close to 100% of projects have this as a dependency anyways).

so instead of object assigning the options one could do something along the lines of:

const hOP = Object.prototype.hasOwnProperty;

export default function (CustomElement, opts = {}) {
  const React = hOP.call(opts, 'React') ? opts.React : defaults.React;
  const ReactDOM = hOP.call(opts, 'ReactDOM') ? opts.ReactDOM : defaults.ReactDOM;

or if falsy check is enought even

export default function (CustomElement, opts = {}) {
  const React = opts.React || defaults.React;
  const ReactDOM = opts.ReactDOM || defaults.ReactDOM;

anyways, probably very premature (:

[treshugart]

We could have babel transpile it (it transpiles it to _extends, but you still have to included the runtime for it, I think). It's a pretty small library, so we're probably fine until full ES6 support lands in the browsers we care about.

[timse]

true 👍