@samitouri / QOS-React / commits / 149860bada

Replace object getter with `Object.defineProperty` in TestRenderer (#10473)

Replace object getter with `Object.defineProperty` in TestRenderer (#10473) * Add babel transform for object getters **what is the change?:** We were not transforming object getters[1], and our new TestRenderer uses one.[2] [1]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/get [2]: https://github.com/facebook/react/blob/master/src/renderers/testing/ReactTestRendererFiberEntry.js#L569 **why make this change?:** Our internal build system was not supporting object getters. **test plan:** `yarn build && yarn test` Also built and opened the 'packaging' fixture. Honestly I'm not sure what else to test, this seems pretty low risk. **issue:** None opened yet * Replace object getter with `Object.defineProperty` in TestRenderer **what is the change?:** Replaces an Object Getter [1] with `Object.defineProperty`. [1]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/get **why make this change?:** Our internal build system does not support object getters. **test plan:** We should do follow-up work to ensure that this doesn't come up again - we can't commit code which uses object getters. **issue:** No issue opened yet * Tweak the Object.defineProperty call in test renderer to make flow pass **what is the change?:** - switched from `Object.defineProperties` to `Object.defineProperty` - set some undefined properties to get flow to pass **why make this change?:** - Flow doesn't seem to play nicely with `Object.defineProperty/ies` calls, specifically when you implement `get` and `set` methods. See https://github.com/facebook/flow/issues/1336 - Switched from `properties` to `property` because it seems more conventional in our codebase to use `property`, and I'm only setting one anyway. **test plan:** `flow` **issue:** no issue * More flow type tweaking **what is the change?:** Forced the typing of an argument to 'Object.defineProperty' to be 'Object' to avoid an issue with Flow. **why make this change?:** To get tests passing and flow passing. **test plan:** `flow && yarn test` **issue:**

Flarnie Marchan committed Aug 17, 2017 at 07:23 UTC 149860bada04642f1af471865805b15709745d62
1 file changed +20 -7
src/renderers/testing/ReactTestRendererFiberEntry.js
+20 -7
@@ -565,13 +565,9 @@ var ReactTestRendererFiber = {
565 invariant(root != null, 'something went wrong');
566 TestRenderer.updateContainer(element, root, null, null);
567
568 - return {
569 - get root() {
570 - if (root === null || root.current.child === null) {
571 - throw new Error("Can't access .root on unmounted test renderer");
572 - }
573 - return wrapFiber(root.current.child);
574 - },
568 + var entry = {
569 + root: undefined, // makes flow happy
570 + // we define a 'getter' for 'root' below using 'Object.defineProperty'
571 toJSON() {
572 if (root == null || root.current == null || container == null) {
573 return null;
@@ -611,6 +607,23 @@ var ReactTestRendererFiber = {
607 return TestRenderer.getPublicRootInstance(root);
608 },
609 };
610 +
611 + Object.defineProperty(
612 + entry,
613 + 'root',
614 + ({
615 + configurable: true,
616 + enumerable: true,
617 + get: function() {
618 + if (root === null || root.current.child === null) {
619 + throw new Error("Can't access .root on unmounted test renderer");
620 + }
621 + return wrapFiber(root.current.child);
622 + },
623 + }: Object),
624 + );
625 +
626 + return entry;
627 },
628
629 /* eslint-disable camelcase */