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

Add a <ReactDOM.Portal /> element #11586

Closed
j-f1 opened this issue Nov 17, 2017 · 6 comments
Closed

Add a <ReactDOM.Portal /> element #11586

j-f1 opened this issue Nov 17, 2017 · 6 comments

Comments

@j-f1
Copy link

j-f1 commented Nov 17, 2017

Do you want to request a feature or report a bug?

I’d like to request a feature.

What is the current behavior?

To create a portal, you currently have to use a function:

function MyComponent(props) {
  return <Foo>
    ...
    {ReactDOM.createPortal(<Bar>
      ...
    </Bar>, myElement)}
  </Foo>
}

What is the expected behavior?

function MyComponent(props) {
  return <Foo>
    ...
    <ReactDOM.Portal target={myElement}>
      <Bar>...</Bar>
    </ReactDOM.Portal>
  </Foo>
}
@sjy
Copy link
Contributor

sjy commented Nov 20, 2017

I would like to make a pr for it, it may take sone time to invesitigate and do some test l.

@swyxio
Copy link
Contributor

swyxio commented Nov 20, 2017

well first of all... do we want it? it definitely looks cleaner but are there any cons?

@gaearon
Copy link
Collaborator

gaearon commented Nov 20, 2017

I do agree it reads a bit better. I think we mostly intended createPortal to be an experimental API at first but then it turned out to be more necessary than we thought, and we changed it to be "stable" at last minute. So we haven't quite fixed the ergonomics.

However there is a bigger issue of portals not being feature-complete. Ideally we want to add support for server rendering in some way (e.g. by letting the container be a string). We also wanted to support cross-renderer portals. We thought that at that point we would move the API into React namespace since it would no longer be DOM specific.

So I think overall I agree this needs to be done, but I'd prefer to do it at the same time as other changes, and just make it React.Portal. I don't think those other changes are trivial though, and will require discussion.

@gaearon
Copy link
Collaborator

gaearon commented Jan 2, 2018

I'm going to close this in favor of #10711.

@gaearon gaearon closed this as completed Jan 2, 2018
@j-f1
Copy link
Author

j-f1 commented Jan 2, 2018

How are these related @gaearon?

@gaearon
Copy link
Collaborator

gaearon commented Jan 2, 2018

From my reply above:

However there is a bigger issue of portals not being feature-complete. Ideally we want to add support for server rendering in some way (e.g. by letting the container be a string). We also wanted to support cross-renderer portals. We thought that at that point we would move the API into React namespace since it would no longer be DOM specific.

So I think overall I agree this needs to be done, but I'd prefer to do it at the same time as other changes, and just make it React.Portal. I don't think those other changes are trivial though, and will require discussion.

#10711 is the issue tracking "isomorphic" portals (at which point we might as well make them more ergonomic to use).

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

No branches or pull requests

4 participants