Remove propTypes object for indirect assignment

[remcohaszing]

This plugin removes the assignment of propTypes on react component, but it doesn't affect the referenced variable.

This means that the definition of the propTypes isn't removed.

For example:

const propTypes = {
  value: PropTypes.string,
};


export default function MyComponent() {
  return <div />;
}


MyComponent.propTypes = propTypes;

Becomes:

{
  value: PropTypes.string,
};


export default function MyComponent() {
  return <div />;
}

This is fine, except when referencing the propTypes from another component.

import MyComponent from './MyComponent';


const propTypes = {
  value: MyComponent.propTypes.value,
};


export default MyOtherComponent(props) {
  return <MyComponent value={props.value} />;
}


MyOtherComponent.propTypes = propTypes;

After processing this becomes:

import MyComponent from './MyComponent';


{
  value: MyComponent.propTypes.value,  // MyComponent.propTypes === undefined
};


export default MyOtherComponent(props) {
  return <MyComponent value={props.value} />;
}

This is a common use case, as this is the recommended way to define propTypes by the airbnb style guide (https://github.com/airbnb/javascript/tree/master/react#ordering)

I suggest to remove (or add an option to remove) the node that creates the propTypes object. So the latter example would simply become:

import MyComponent from './MyComponent';


export default MyOtherComponent(props) {
  return <MyComponent value={props.value} />;
}

When using wrap mode, the indirect assignment should be turned into a direct assignment. So the first example would be transformed into:

export default function MyComponent() {
  return <div />;
}


if (process.env.NODE_ENV !== 'production') {
  MyComponent.propTypes = {
    value: PropTypes.string,
  };
}

As a side effect, the bundle will become even smaller. 😄

[oliviertassinari]

This is fine, except when referencing the propTypes from another component.

That's expected. I have added a section in the documentation for that.
It's a duplicate of #70.

[oliviertassinari]

The path, I think we should follow, is discouraging users recycling the propTypes by accessing them indirectly.

[oliviertassinari]

As a side effect, the bundle will become even smaller.

Actually, I'm wondering, why is this an issue at all. Isn't it a dead code that uglify should identify and remove?

[remcohaszing]

I expected this as well, but apparently it doesn't.

*example.js*

const PropTypes = {};
const MY_CONSTANT = 'MY_CONSTANT';
const MyComponent = {};
const propTypes = {
  foobar: PropTypes.string
};


if (process.env.NODE_ENV !== 'production') {
  MyComponent.propTypes = propTypes;
}

export default MyComponent;

*package.json*

{
  "name": "webpack-bug",
  "scripts": {
    "build": "webpack -p"
  },
  "devDependencies": {
    "babel-core": "^6.22.1",
    "babel-loader": "^6.2.10",
    "babel-preset-es2015": "^6.22.0",
    "webpack": "^2.2.0"
  },
  "babel": {
    "presets": [
      "es2015"
    ]
  }
}

*webpack.config.js*

const path = require('path');


module.exports = {
  entry: path.join(__dirname, 'example.js'),
  output: {
    filename: 'out.js',
    path: 'out'
  },
  module: {
    rules: [
      {
        test: /\.js$/,
        loader: 'babel-loader',
      },
    ],
  },
  devtool: 'source-map',
};

*out/out.js*

!function(e){function r(n){if(t[n])return t[n].exports;var o=t[n]={i:n,l:!1,exports:{}};return e[n].call(o.exports,o,o.exports,r),o.l=!0,o.exports}var t={};return r.m=e,r.c=t,r.i=function(e){return e},r.d=function(e,t,n){r.o(e,t)||Object.defineProperty(e,t,{configurable:!1,enumerable:!0,get:n})},r.n=function(e){var t=e&&e.__esModule?function(){return e.default}:function(){return e};return r.d(t,"a",t),t},r.o=function(e,r){return Object.prototype.hasOwnProperty.call(e,r)},r.p="",r(r.s=0)}([function(e,r,t){"use strict";Object.defineProperty(r,"__esModule",{value:!0});var n={},o={};({foobar:n.string});r.default=o}]);
//# sourceMappingURL=out.js.map

As seen in the output, an object is still created having a foobar property (near the end of out.js.

Additionally,

[lencioni]

@oliviertassinari I'm seeing the same behavior. I think one cause of the unused variable not being removed is that if it contains any function calls the minifier won't remove it in case there are side-effects to the function calls.

I have noticed the additional problem that when using the removeImport option, the imports are also not removed since they are still referenced in the propTypes variable. So even if the minifier was removing the unused variable, it will not remove the imports.

I think this plugin should be enhanced so that it transforms code that looks like this:

import React from 'react';
import PropTypes from 'prop-types';

const propTypes = {
  children: PropTypes.node,
};

export default class MyComponent extends React.Component {
  render() {
    return <div>{this.props.children}</div>;
  }
}

MyComponent.propTypes = propTypes;

into this:

import React from 'react';

export default class MyComponent extends React.Component {
  render() {
    return <div>{this.props.children}</div>;
  }
}

At least for the simple case when it can be determined that the variable is no longer used.

Additionally, we often wrap our propTypes in a function when assigning it to the component, like this:

import { forbidExtraProps } from 'airbnb-prop-types';

const propTypes = {
  // ...
};

class MyComponent extends React.Component {
  // ...
}

MyComponent.propTypes = forbidExtraProps(propTypes);

So the unused variable detection would ideally be smart enough to transform the above to:

class MyComponent extends React.Component {
  // ...
}

What do you think? Is it worth taking a swing at this?

[lencioni]

I opened up #145 to continue the discussion