Skip portals when rendering on server-side.

[diasbruno]

Hi,

Here is an option that just skips portals while rendering on the server-side environment.

I'll try to find some ideas to help dealing with modals on tests.

Please let me know if there are any improvements in this PR.
If the solution is not ideal, it can be closed.

Thank you.

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[cpojer]

Would you mind rebasing this PR on top of master? Then we can figure out how to merge this :)

[diasbruno]

@cpojer Done.

[sizebot]

React: size: 🔺+0.7%, gzip: 🔺+0.3%

Details of bundled changes.

Comparing: cc5a493...2509074

react

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
react.development.js	+0.3%	+0.2%	106.9 KB	107.27 KB	27.03 KB	27.09 KB	UMD_DEV
react.production.min.js	🔺+0.7%	🔺+0.3%	12.01 KB	12.09 KB	4.64 KB	4.66 KB	UMD_PROD
react.profiling.min.js	+0.6%	+0.3%	14.17 KB	14.25 KB	5.18 KB	5.2 KB	UMD_PROFILING
react.development.js	+0.5%	+0.3%	69.65 KB	70.02 KB	17.95 KB	17.99 KB	NODE_DEV
react.production.min.js	🔺+1.2%	🔺+0.5%	6.44 KB	6.52 KB	2.67 KB	2.69 KB	NODE_PROD
React-dev.js	+0.6%	+0.3%	67.92 KB	68.31 KB	17.29 KB	17.34 KB	FB_WWW_DEV
React-prod.js	🔺+1.0%	🔺+0.7%	16.02 KB	16.19 KB	4.24 KB	4.27 KB	FB_WWW_PROD
React-profiling.js	+1.0%	+0.7%	16.02 KB	16.19 KB	4.24 KB	4.27 KB	FB_WWW_PROFILING

Generated by 🚫 dangerJS

[gaearon]

I don't think we want to skip them. You can't render a portal without a DOM container element regardless. Therefore, the application code should skip them anyway.

The original behavior was intentional, and it's not clear to me why it is being removed here.

[diasbruno]

This is an old patch. Not sure if it is relevant now or there are things to change. I believe most implementations that uses createPortal have some kind of check if DOM is available.

IMHO, since portals can't be rendered on server-side, I don't know if throwing an exception is desired and will make all libraries to check for DOM environment. Making portal a no-op seems ok. Perhaps, display a warn when trying to render?

cc @cpojer @gaearon

[oliviertassinari]

To provide more context, one of the reasons why Material-UI has a Portal component is to avoid the server-side error.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[stale]

Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you!