Annotate pure calls with babel plugin to slightly improve tree-shakea…

[Andarist]

…bility

Q	A
Fixed Issues?	Fixes #84,
Documentation only PR
Patch: Bug Fix?
Minor: New Feature?	👍
Major: Breaking Change?
Tests Added + Pass?	Yes

Truth to be told only 2 packages at the moment benefit from it.

redux-subspace-loop - namespaced export is created by a call and it might get dropped if the package stays unused

react-redux-subspace - SubspaceProvider gets correctly annotated, but it cannot be removed because it uses static properties, so the output looks like this:

var SubspaceProvider = /*#__PURE__*/function (_React$PureComponent) {
 // ...
}(React__default.PureComponent);

SubspaceProvider.propTypes = {
    children: PropTypes.element.isRequired,
    mapState: /*#__PURE__*/PropTypes.oneOfType([PropTypes.func, PropTypes.string]),
    namespace: PropTypes.string,
    subspaceDecorator: PropTypes.func
};

SubspaceProvider.contextTypes = {
    store: PropTypes.object.isRequired
};

SubspaceProvider.childContextTypes = {
    store: PropTypes.object
};

Unfortunately such static assignments prevent dead code removal, because algorithm thinks that SubspaceProvider is used.

There are 2 ways of "fixing" this problem:

• custom babel plugin to pull those static assignments into the transpiled class IIFE, I might create it some day, but in general would like to fix it upstream - I have working PR for this in babel@7, but it's not merged in et
• create this class from the IIFE, like this:

const SubspaceProvider = (() => {
  class SubspaceProvider extends React.PureComponent {
    // ...
  }
  return SubspaceProvider
})()
export default SubspaceProvider

this will let the plugin annotate outer IIFE (inner being a transpiled class) which would contain whole class definition and thus making it dead code droppable.

But ofc this is somewhat ugly and also gives rather rly small benefits - in real world scenarios for this library probably even none.

[mpeyper]

As long the core package (redux-subspace) gets some benefit, I'm happy. That's the one with the most stuff that isn't going to be used by everyone.

I'll try and find some time tonight to take a closer look, but on the surface everything looks fine here.

[Andarist]

well, redux-subspace does not benefit from this at the moment, because nothing in top level scope is created with a function call - so regular tree-shaking should be able to remove unused stuff anyway

this can ofc change in the future, so it might be good to leave the plugin just in case

[mpeyper]

redux-subspace does not benefit from this at the moment, because nothing in top level scope is created with a function call

hmmm, ok. I think I understand a bit better what this plugin doing now. When a variable is assigned from a function call just by importing a file, this tells the tree shaker that is was a pure call and can be removed. Without it, the variable can be shaken away, but the call, and consequently the function itself, are left behind as it may have had a side-effect worth keeping. Am I close?

SubspaceProvider gets correctly annotated, but it cannot be removed because it uses static properties

Changing SubspaceProvider to

class SubspaceProvider extends React.PureComponent {
  static get propTypes() {
    return {
      children: PropTypes.element.isRequired,
      mapState: PropTypes.oneOfType([PropTypes.func, PropTypes.string]),
      namespace: PropTypes.string,
      subspaceDecorator: PropTypes.func
    }
  }

  static get contextTypes() {
    return {
      store: PropTypes.object.isRequired
    }
  }

  static get childContextTypes() {
    return {
      store: PropTypes.object
    }
  }

  getChildContext() {
    const makeSubspaceDecorator = props => props.subspaceDecorator || subspace(props.mapState, props.namespace)

    return {
      store: makeSubspaceDecorator(this.props)(this.context.store)
    }
  }

  render() {
    return Children.only(this.props.children)
  }
}

outputs

var SubspaceProvider = /*#__PURE__*/function (_React$PureComponent) {
  inherits(SubspaceProvider, _React$PureComponent);

  function SubspaceProvider() {
    classCallCheck(this, SubspaceProvider);
    return possibleConstructorReturn(this, _React$PureComponent.apply(this, arguments));
  }

  SubspaceProvider.prototype.getChildContext = function getChildContext() {
    var makeSubspaceDecorator = function makeSubspaceDecorator(props) {
      return props.subspaceDecorator || subspace(props.mapState, props.namespace);
    };

    return {
      store: makeSubspaceDecorator(this.props)(this.context.store)
    };
  };

  SubspaceProvider.prototype.render = function render() {
    return Children.only(this.props.children);
  };

  createClass(SubspaceProvider, null, [{
    key: 'propTypes',
    get: function get$$1() {
      return {
        children: PropTypes.element.isRequired,
        mapState: PropTypes.oneOfType([PropTypes.func, PropTypes.string]),
        namespace: PropTypes.string,
        subspaceDecorator: PropTypes.func
      };
    }
  }, {
    key: 'contextTypes',
    get: function get$$1() {
      return {
        store: PropTypes.object.isRequired
      };
    }
  }, {
    key: 'childContextTypes',
    get: function get$$1() {
      return {
        store: PropTypes.object
      };
    }
  }]);
  return SubspaceProvider;
}(React.PureComponent);

which I think means it can be shaken away, but I'm not sure if that's necessarily a recommended (or valid?) way to define a React component?

this can ofc change in the future, so it might be good to leave the plugin just in case

I like the idea of helping future me (and others) out and leaving it in for all packages, even if they don't get any immediate benefit from it right now.

[Andarist]

hmmm, ok. I think I understand a bit better what this plugin doing now. When a variable is assigned from a function call just by importing a file, this tells the tree shaker that is was a pure call and can be removed. Without it, the variable can be shaken away, but the call, and consequently the function itself, are left behind as it may have had a side-effect worth keeping. Am I close?

You couldn't be closer :)

which I think means it can be shaken away, but I'm not sure if that's necessarily a recommended (or valid?) way to define a React component?

Neat trick! didn't think of that, although I'm not sure if using getters for those is a good thing.

I like the idea of helping future me (and others) out and leaving it in for all packages, even if they don't get any immediate benefit from it right now.

👍

[mpeyper]

I'm not sure if using getters for those is a good thing.

No, me neither. I don't think I'll worry with it.

The reality is, whether you use subspaced or SubspaceProvider from that package, you will need that code, so it's unlikely it would ever have been removed if the package was used.

I'm happy with this and will merge it now. Thanks again :)