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

[fixed] don't access ReactDOM.createPortal if DOM not available #825

Merged

Conversation

keeganstreet
Copy link

Fixes #824

Changes proposed:

  • Access ReactDOM.createPortal via a function instead of directly, so it is only accessed when required, not when the script is compiled. Stops errors being thrown when bundled into script that is run in an environment without access to ReactDOM.

Upgrade Path (for changed or removed APIs):

N/A

Acceptance Checklist:

  • The commit message follows the guidelines in CONTRIBUTING.md.
  • (N/A) Documentation (README.md) and examples have been updated as needed.
  • (N/A) If this is a code change, a spec testing the functionality has been added.
  • (N/A) If the commit message has [changed] or [removed], there is an upgrade path above.

@coveralls
Copy link

coveralls commented Aug 19, 2020

Coverage Status

Coverage remained the same at 82.632% when pulling 024b640 on keeganstreet:lazy-access-of-react-dom-createportal into 421a1c8 on reactjs:master.

@diasbruno
Copy link
Collaborator

Maybe something like this will work: const isReact16 = canUseDOM && ReactDOM.createPortal !== undefined;.
If we don't have browser-like env, we don't render the modal anyways...

What do you think?

@keeganstreet
Copy link
Author

@diasbruno sounds good, that would require fewer changes. I have just tested that and it will resolve the problem for us. I'll push an update to this branch.

@keeganstreet keeganstreet force-pushed the lazy-access-of-react-dom-createportal branch from 38f6862 to 024b640 Compare August 19, 2020 11:37
@diasbruno diasbruno self-requested a review August 19, 2020 11:38
@diasbruno diasbruno merged commit 94ad567 into reactjs:master Aug 19, 2020
@diasbruno
Copy link
Collaborator

Thanks, @keeganstreet!!

@diasbruno diasbruno changed the title [fixed] lazily access ReactDOM.createPortal [fixed] don't access ReactDOM.createPortal if DOM not available Aug 19, 2020
@diasbruno
Copy link
Collaborator

I've updated the title so it's easy to find.

@keeganstreet
Copy link
Author

Not a problem @diasbruno :-) Thanks for your help!

Do you have a release schedule for npm? Wondering when we'll be able to have this fix for our system or if we need to put in a temporary workaround?

@keeganstreet keeganstreet deleted the lazy-access-of-react-dom-createportal branch August 19, 2020 23:29
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

Successfully merging this pull request may close these issues.

Bug: React Modal is using ReactDOM on the server
3 participants