Is this addon expected to ship in production?

[cibernox]

I have read through the code and seems that this addon is unconditionally including the proptypes library and reopening components, unless I've missed something.

Is this intended? I think it should only run in development and tests but be removed in production.

Disclaimer: i've never used PropTypes in react, so I don't know if there is any reason to run it in production too.

[evrowe]

You're correct, currently this will all ship in production builds. Right now it should be possible to blacklist the addon for prod in an app's `ember-cli-build.js`, however I will be shipping an update in the next day or two that will ensure both the library and the component reopening are only included in development builds. Thanks for filing this!

…

On Thu, May 18, 2017 at 06:22 Miguel Camba ***@***.***> wrote: I have read through the code and seems that this addon is unconditionally including the proptypes library and reopening components, unless I've missed something. Is this intended? I think it should only run in development and tests but be removed in production. Disclaimer: i've never used PropTypes in react, so I don't know if there is any reason to run it in production too. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOWfVuWCWnWERaxAQAUXrCU3vmaKO_vks5r7EYGgaJpZM4NfN5_> .

[evrowe]

This is now resolved in 1.1.0. I would love some feedback on the approach used to handle removing the code for prod builds; while the approach I have works, I suspect there is a better way to handle this.

[asvny]

How about a babel-plugin which removes proptypes and import statements when env is in production mode ?

[cibernox]

I summon @rwjblue since he has experience with removing code in production.

Do you know any good example we can copy from to get this library, and the propTypes of each component, completely removed in production?

[asvny]

I was referring something related like this https://github.com/oliviertassinari/babel-plugin-transform-react-remove-prop-types

[evrowe]

@asvny Thanks for the suggestion, I'll definitely explore that as an option. Could be much simpler/easier to utilize than my current approach.

Would definitely appreciate some input from @rwjblue on this matter as well, I'm sure he's got some great insight. Thanks for thinking to mention him, @cibernox

[DHedgecock]

@cibernox the prop-types library has a prop-types.js and a prop-types.min.js. The .min file is only 2KB (0.8KB gzipped). I'll be merging in PR #2 soon, which imports the .min file for production builds.

The Component.reopen will be stripped out in production builds (the initializer is still run).

I am planning on implementing a getDefaultProps to allow for default props on Components with optional passed properties, where the component is always declared in template with the property passed, but a value may only be passed some of the time (this is not a common case, but we've run into it in our application a few times, especially when using the component helper with dynamic passed property values 😅 ).

B/c of incoming getDefaultProps, which should be run in production, this addon is intended to be included in production builds, but the prop-types validators will only be imported in dev builds and the call to checkPropTypes will also only occur in dev builds.

@asvny would love to use a babel plugin 👍 , have any interest in porting over the babel-plugin-transform-react-remove-prop-types to work with Ember apps?

Thanks for your interest in the addon, we're very excited about borrowing the React props validation patterns into our Ember applications. Keep the ideas coming!

[asvny]

@DHedgecock Much happy to help !

I'm am very interested in using this plugin because in a glance I can know what all are props it takes and shape of it (kinda self documenting).

[DHedgecock]

Alright, #2 is merged, which upgrades the dev vs prod imports. I also added some notes to the README here: https://github.com/healthsparq/ember-cli-prop-types#in-production.

tldr:

• Yes, this addon needs to be included for prod builds or your propType definitions and imports will throw errors
• In production the addon will import a very lightweight set of AMD shims. Application is not altered.
• Addition of getDefaultProps that will run in production is planned.

I'm going to close this issue, feel free to add additional comments if you have additional concerns. And thanks again for all the scrutiny. Feels good to get all the ducks lined up 👍 .

[evrowe]

🙌🙌🙌🙌

…

On Mon, May 22, 2017 at 16:19 Dan Hedgecock ***@***.***> wrote: Alright, #2 <#2> is merged, which upgrades the dev vs prod imports. I also added some notes to the README here: https://github.com/healthsparq/ember-cli-prop-types#in-production. tldr: - Yes, this addon needs to be included for prod builds or your propType definitions and imports will throw errors - In production the addon will import a very lightweight set of AMD shims. Application is not altered. - Addition of getDefaultProps that will run in production is planned. I'm going to close this issue, feel free to add additional comments if you have additional concerns. And thanks again for all the scrutiny. Feels good to get all the ducks lined up 👍 . — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#1 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOWfVDB4bmjLGagHAAPU9bjbwMX35Goks5r8hfngaJpZM4NfN5_> .