Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

propTypes not removed for HOC #106

Closed
LeonardoGentile opened this issue May 3, 2017 · 5 comments
Closed

propTypes not removed for HOC #106

LeonardoGentile opened this issue May 3, 2017 · 5 comments

Comments

@LeonardoGentile
Copy link

LeonardoGentile commented May 3, 2017

Hello,
I've just installed this and it seems that for my edge cases this is not working.

The following propTypes are not removed from the build. While all the others declared in the standard way are correctly removed.

CASE 1:

const myPropName = 'whatever';
BaseLink.propTypes = {
  prop1: PropTypes.string
  prop2: PropTypes.node
};

BaseLink.propTypes[myPropName] = PropTypes.object;  //<== This is not removed

CASE 2:

const myPropName = 'whatever';
// Both are not removed
MyComp.wrappedComponent.propTypes = {};
MyComp.wrappedComponent.propTypes[myPropName] = PropTypes.object.isRequired;

Here I used wrappedComponents, because I'm decorating my original component MyComp with mobx-react @observer that will indeed create a new wrapper. So the original component is available as MyComp.wrappedComponent.

CASE 3:
This applies to both cases above.
I'm importing my PropTypes from the new separate packages

import PropTypes from 'prop-types';

It seems the import is never removed from the build using the default config (supposed to be remove mode).

I'm not sure if this is due to the 2 cases above (still having some propTypes in the build would not allow to remove the import?) or because I import them from the separate package and not from the React core... 🤔

@oliviertassinari
Copy link
Owner

oliviertassinari commented May 3, 2017

I would need the full source code of your examples. CASE 1 and CASE 2 are expected from the source code you are sharing. For CASE 3, you could try the removeImport option.

@LeonardoGentile
Copy link
Author

@oliviertassinari thanks for the quick reply!
Case1: https://github.com/LeonardoGentile/react-mobx-router5/blob/dev/src/modules/BaseLink.js
Case2: https://github.com/LeonardoGentile/react-mobx-router5/blob/dev/src/modules/routeNode.js

Notice that your plugin is not in that branch yet cause I'm still experimenting with it right now.
For case 3 I'll let you know asap

@LeonardoGentile
Copy link
Author

LeonardoGentile commented May 3, 2017

Concerning CASE 3, I've tried multiple times in my babel config:

 "plugins": [
      ["transform-react-remove-prop-types", {
         "mode": "remove",
         "removeImport": true
       }],
      "transform-decorators-legacy",
      "transform-runtime"
    ]

but it seems to have no effect. The import is never removed.

@LeonardoGentile
Copy link
Author

Also this won't work:

BaseLink.propTypes[storeName] /* remove-proptypes */ = PropTypes.object;
BaseLink.propTypes['route'] /* remove-proptypes */ = PropTypes.object;
BaseLink.propTypes['previousRoute'] /* remove-proptypes */ = PropTypes.object;
BaseLink.propTypes['isActive'] /* remove-proptypes */ = PropTypes.bool;
// ...
RouteNode.wrappedComponent.propTypes /* remove-proptypes */ = {};
RouteNode.wrappedComponent.propTypes[storeName] /* remove-proptypes */ = PropTypes.object.isRequired;

LeonardoGentile added a commit to LeonardoGentile/react-mobx-router5 that referenced this issue May 5, 2017
@oliviertassinari
Copy link
Owner

Here is a simplified failing case to illustrate #108.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants