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: Option to remove imports of custom propTypes in current package, and from deep imports #144

Open
lencioni opened this issue Jul 11, 2018 · 3 comments

Comments

@lencioni
Copy link
Collaborator

We have some packages that have some custom propTypes defined in modules in that package itself. For example:

import customPropType from '../propTypes/customPropType';

These custom propTypes can also be imported by consuming packages via deep imports:

import customPropType from 'special-package/propTypes/customPropType';

It would be nice to have an option that is similar to additionalLibraries that could be used to remove these imports. Perhaps the option could take a regex pattern or glob pattern?

@oliviertassinari
Copy link
Owner

oliviertassinari commented Jul 11, 2018

@lencioni What makes you think this plugin can be more efficient at removing dead code than another tool specifically tailored for it? I mean, if a dead code prunner can't remove some code, what enable us, from an architectural point of view, to do it?

We definitely want people to have a bundle prop-types free when using a JavaScript minifier + this plugin. I have been experiencing the same issue in the past. I have been using hacks like this one to save bundle size.

@lencioni
Copy link
Collaborator Author

Although the minification of the custom propType code itself, as you linked to, is useful, I would ideally like to remove the import as well in the same way that the additionalLibraries option works, not only to save bundle size, but also to improve runtime speed a little by removing the overhead of an additional module everywhere.

I think a JavaScript minifier will not remove the unused import because it doesn't know that the import is free of side-effects. Having a way to opt in related imports in this plugin would be useful.

Another thought that occurs to me is that this could perhaps be automatically detected. When removing the propTypes, it could detect any newly unused imports and remove them as well.

@oliviertassinari
Copy link
Owner

oliviertassinari commented Jul 11, 2018

I don't think that it should be the default behavior given the potential side effects you mentioned, at the exception of prop-types that is side-effect free, so we can do it automatically.
Still, I agree. We have the opportunity with this plugin to efficiently remove the imports.

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

No branches or pull requests

2 participants