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

[react-popper] Uncaught ReferenceError: process is not defined #1579

Open
nijk opened this issue Apr 13, 2018 · 9 comments
Open

[react-popper] Uncaught ReferenceError: process is not defined #1579

nijk opened this issue Apr 13, 2018 · 9 comments

Comments

@nijk
Copy link
Contributor

nijk commented Apr 13, 2018

When attempting to use react-popper with Closure Compiler's :optimizations :advanced I get a javascript error that process is not defined. I believe this is because ReactPopper uses the transform-react-remove-prop-types babel plugin which wraps the PropTypes in a conditional referencing process.env.NODE_ENV to allow for Dead Code Elimination (DCE). This doesn't seem to be appropriate for a production build as the Dead Code is not actually eliminated from the minified asset and it relies on a global which is not normally available in production React environments.

This may be an issue for the ReactPopper dependency to resolve, but I wanted to highlight it for others here and get an understanding of the options for resolving this.

@EmergentBehavior I noticed that you recently updated the react-popper package, did you experience this issue locally? Any tips or suggestions would be greatly appreciated

@EmergentBehavior
Copy link
Contributor

I did run into this issue as well, unfortunately. My short term solution was to define process ahead of the loading of my compiled application JS:

<script>var process = {env: {}};</script>

Additionally, you could define NODE_ENV within if you desired.

Longer term solution might be to (a) notify the developer or (b) create a new patch release of react-popper here that builds without this.

@nijk
Copy link
Contributor Author

nijk commented Apr 13, 2018

Ok, well I'm feeling a little more vindicated in my struggles with this particular problem as I came to the same conclusion.

I think there is one more solution and that is to create a webpack config in the cljsjs package that builds react-popper from source without the transform-react-remove-prop-types babel plugin. The build function could then shell out to npm install using the --ignore-scripts flag and call its own build process using the webpack config.

I wonder if a) is the right solution with c) being the temporary stop-gap. I am slightly concerned that as ReactPopper has moved onto 1.x releases, change requests to 0.x may not be a priority. That said, the maintainer is very responsive to GH issues

@nijk
Copy link
Contributor Author

nijk commented Apr 13, 2018

@EmergentBehavior Out of interest, do you know if it is possible to dynamically define process.env.NODE_ENV with :optimizations :advanced - myself and a colleague tried multiple ways to get it working but we couldn't get process to be defined as a global even when we supplied process as an extern.

The use case here is about: working around this issue and having the PropType warnings behave correctly in both development & production builds.

@EmergentBehavior
Copy link
Contributor

I think there is one more solution and that is to create a webpack config in the cljsjs package that builds react-popper from source without the transform-react-remove-prop-types babel plugin. The build function could then shell out to npm install using the --ignore-scripts flag and call its own build process using the webpack config.

I agree. Might be worth doing this and then opening an issue with https://github.com/FezVrasta/react-popper to nudge them to fix it.

Out of interest, do you know if it is possible to dynamically define process.env.NODE_ENV with :optimizations :advanced - myself and a colleague tried multiple ways to get it working but we couldn't get process to be defined as a global even when we supplied process as an extern.

For our application, we dynamically generate the index.html files depending on environment, so you could create a template and then selectively include the process definition if it's a production build.

You could also try selectively running (if prod build) something like (set! js/process {}) as long as it's defined before the popper dependency is loaded.

There might be a more efficient way but I can't think of it off top of my head.

@nijk
Copy link
Contributor Author

nijk commented Apr 13, 2018

Thanks @EmergentBehavior, I appreciate your input and especially the quick response.

I'm planning to raise a webpack build PR on react-popper in the next few days, then I'll raise an issue on their repo.

@EmergentBehavior
Copy link
Contributor

EmergentBehavior commented Apr 15, 2018

@nijk I'm also getting an intermittent error around Uncaught TypeError: Cannot read property 'placements' of undefined -- are you?

@EmergentBehavior
Copy link
Contributor

I think because they call (in unminifed file) var placements = PopperJS.placements; and PopperJS is undefined.

@EmergentBehavior
Copy link
Contributor

The issue was a corrupted PopperJS build (because of the bundled popperutils), see #1582 and #1583

@nijk
Copy link
Contributor Author

nijk commented Apr 24, 2018

Sorry for the delay on this, I've gone down a bit of a rabbit hole with this issue in ReactPopper which has resulted in finding an upstream issue in the babel-plugin-transform-react-remove-prop-types lib - this lib doesn't seem to be actively maintained as my issue has gone for 5 days without reply so I'm going to pursue this approach in the cljsjs package for ReactPopper:

Create a webpack config in the cljsjs package that builds react-popper from source without the transform-react-remove-prop-types babel plugin. The build function could then shell out to npm install using the --ignore-scripts flag and call its own build process using the webpack config.

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

No branches or pull requests

2 participants