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

Feature request: remove unused propTypes intermediate variable #145

Closed
lencioni opened this issue Jul 11, 2018 · 1 comment · Fixed by #146
Closed

Feature request: remove unused propTypes intermediate variable #145

lencioni opened this issue Jul 11, 2018 · 1 comment · Fixed by #146

Comments

@lencioni
Copy link
Collaborator

This is a request for a feature similar to #75.

Pulling this comment out from #75 for visibility:

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
Copy link
Collaborator Author

@oliviertassinari I've added some failing test cases for this here: #146

Do you have any thoughts about this?

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