Skip to content
This repository has been archived by the owner on Dec 5, 2024. It is now read-only.

v0.10.1 - SSR breaks because of React-Popper + No /dist builds in 1.0beta5 #145

Closed
virgofx opened this issue Apr 20, 2018 · 5 comments
Closed

Comments

@virgofx
Copy link

virgofx commented Apr 20, 2018

The UMD build is broken in v0.10.1 because of:

Manager.propTypes = process.env.NODE_ENV !== "production" ? {

The production build should use plugin that optimizes and removes this code ahead of time. In Webpack it was the Define plugin. Then the remove prop-types plugin will correctly wrap it and leave it as {}.

What's the best way to resolve this?

0.10.1

Rollup. I can find the corresponding plugin to ensure it optimizes it out and submit a PR for an immediate patch fix. Or are you not releasing updates anymore?

1.x.x

What's the situation here? There's currently no /dist. This is a huge problem. Reactstrap utilizes external opt-in dependency for Bootstrap and is a huge community. https://github.com/reactstrap/reactstrap

Ref: #142

virgofx referenced this issue in reactstrap/reactstrap Apr 20, 2018
@nijk
Copy link

nijk commented Apr 21, 2018

@virgofx for v0.10.1 see this issue I've raised with babel-plugin-transform-react-remove-prop-types.

Basically the default mode that is supposed to entirely remove the propTypes from production builds is not working for React stateless functional components and therefore I can't fix issue #142. As you've found in this issue process.env.NODE_ENV is not available in the client (browser or node) unless it is provided by something like the DefinePlugin in Webpack or similar in your own bundling process. This is a requirement that I'm not keen on either, but for now I would suggest that you do one of two things:

  1. Find a way to provide the process global in your application and ensure .env.NODE_ENV is set correctly for your build environments.
  2. Raise a PR to entirely remove the babel-plugin-transform-react-remove-prop-types plugin's usage

@virgofx
Copy link
Author

virgofx commented Apr 21, 2018

I'm not sure about the stateless components; however, the issue with regards to process can be fixed quite easily. I'll submit a PR which fixes this.

@nijk
Copy link

nijk commented Apr 21, 2018

@virgofx Try following the replication steps I've posted on that ticket about stateless functional components and you'll understand. The point is that babel-plugin-transform-react-remove-prop-types should work properly, but it doesn't because of a bug with removing propTypes from functional components (ReactPopper uses two of them), which means we can't just remove {mode: 'wrap'} from the plugin config on v0.x and expect all propTypes to be removed.

FYI: The {mode: 'wrap'} config is what actually adds this conditional:
Manager.propTypes = process.env.NODE_ENV !== "production" ? {

The intended default behaviour of the plugin is to remove propType declarations entirely

@virgofx
Copy link
Author

virgofx commented Apr 21, 2018

So seems to fix

  1. Continue to use mode: 'wrap' but add the Rollup replace plugin -- which will clean that all up ... or
  2. Just switch to "removeImport": true

Regardless, the Rollup replace plugin (equivalent of Webpack's define) should be used regardless so my thought is to just leave as mode:wrap which is intended for libraries that may be consumed externally.

(Minus any areas with SFC that don't work) ... Regardless, server rendering should be fixed after that, correct?

I added the rollup replace and got the process.env.NODE_ENV all removed and it works fine in my local environment.

@FezVrasta
Copy link
Member

Fixed in 0.10.2

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

No branches or pull requests

3 participants