Remove Prop-Types from Production Build

[virgofx]

It would be nice if PropTypes wasn't in the production build in any fashion. I believe this could be achieved by utilizing the following plugin:

https://www.npmjs.com/package/babel-plugin-transform-react-remove-prop-types

This would allow using the library without having to include the prop-types library as a pre-requisite in production.

[FezVrasta]

That would be cool, feel free to send a PR also!

[Justkant]

Hi,
If you're talking about the umd build, I think that the prop-types module is mark as external and so not included there.
But if not, I don't think that the library should do that but you clearly should use this plugin for your app in production.

As said in #91
Recommended way for libraries: https://github.com/facebook/prop-types#how-to-depend-on-this-package
Also: facebook/prop-types#44

[virgofx]

1. I am referring to the UMD build.
2. You are wrong in your assumptions on how external works. That refers to whether the referenced library is included.

This PR is in regards to the use of Prop-Types ... it should make use of the remove prop types plugin as numerous other application libraries do allowing inheriting apps to not have a required dependency in production. Making a peer dependency only impacts local development and is NOT correct as properly referenced.

[Justkant]

Got you, so we should strip prop-types for the minified version of the umd build ?

[virgofx]

Yes. Not necessarily "strip" but use the proper mode (I believe wrap) with the plugin to remove as much as possible and where appropriate transforms certain situations to {} where constants dot-walks won't fail. ... Effectively removing as much as possible such that a referenced application does not need to manually include prop-types for this to work.

[Justkant]

Ok the need to include prop-types is there because we use externals with the webpack conf, by removing prop-types from the externals you don't need to manually include it anymore.
But the size of the bundle is bigger even in the production build (I thought that uglify could remove the dead_code and so the prop-types).
What would be the prefered result here ? Not having to include prop-types using the umd build even with a bigger module size ?

[virgofx]

The original issue outlines everything in detail. Please read up on how things work.

1 - prop-types should be listed as external. This allows consuming applications to handle as necessary and the distribution builds will EXCLUDE the prop-types library. This is desired.

2 - However, References to prop types inside the library are now unnecessary and have references to constants used by the prop-types library. This is not needed in production as it requires an additional include for applications that don't already include the prop-types in production (most don't). Additionally, it results in extra code and a larger dist size.

Please read up on prop types, before continuing any further in this thread.

[Justkant]

I understand that, I already took some time yesterday to add the babel-plugin that I also use on almost all my react projects.

The part were I'm a bit confused is related to the use of the UMD build. (Maybe an example could help me there)

Also if you can find a repo that is already setup the way you want please share it because on all the repos that I looked at, no one was making the prop-types as external.

EDIT: I build my own little example to fix your issue

[Justkant]

A problem for this lib would be this issue
One solution would be to handle contextTypes & childContextTypes ourself, like so:

Arrow.contextTypes = process.env.NODE_ENV !== "production" ? {
  popper: PropTypes.object.isRequired,
} : {
  popper: () => {},
}

[FezVrasta]

What if we simply wait for the new context API? Once we get it we'll be able to get rid of PropTypes all together

[Justkant]

@FezVrasta yeah I thought about waiting for the new context API too, but if @virgofx doesn't want to wait #94 is still a solution.

I wonder if after the refactoring for the new context we'll be able to support the old context.
An idea on this subject too ?

[virgofx]

@Justkant The plugin prevents writing code which you did. I haven't had time to test; however, it's literally as simple as marking prop-types as external, and using the preset for all builds with appropriate flags (mode:wrap). It still should leave in context and a few other areas (That's what needs to be tested).

I haven't checked out the code in this library yet ... or it's dependencies... If moving to next context (and this uses context) I believe that would make the deps require React 16.[x]? which may or may not be an issue. That may need to be investigated. Regardless, the prop-types plugin should still be used regardless of context use.

Sample Library

• Material-UI

[Justkant]

@virgofx You're right and if you take a look at #94 you can see the prop-types plugin in use. (with the mode: wrap)

Also for the contextTypes & childContextTypes that are not handle by the plugin, I followed the test of someone else

I also tested everything from jsfiddle to be sure it was working without the prop-types (new vs old -- look at the console)

🤣Material-UI was one of the lib that I looked at but they're not using externals for prop-types (umd webpack conf)
That's why I was asking for it in the first place.