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

Critical: Concurrent renders #5

Open
CosticaPuntaru opened this issue Sep 3, 2018 · 4 comments · May be fixed by #11
Open

Critical: Concurrent renders #5

CosticaPuntaru opened this issue Sep 3, 2018 · 4 comments · May be fixed by #11
Labels

Comments

@CosticaPuntaru
Copy link

on the server side there can be more then one requests at the same time being processed,

the const portals = []; at the file level will be shared between requests, and the output won't be what is expected.

in order to work properly the portals array needs to be sandboxed per request.

@MichalZalecki
Copy link
Owner

@CosticaPuntaru That's a valid point, thanks. It would be better when the first call of appendUniversalPortals would create a scope limited to the single request and then inject the markup.

@MioQuispe
Copy link

MioQuispe commented Sep 29, 2018

I'm a fan of this, but perhaps add a big fat disclaimer not to use this until this issue is fixed? Only saw this by chance and it's a dealbreaker tbh.

@MichalZalecki
Copy link
Owner

@MioQuispe Sure, thanks.

@MioQuispe
Copy link

@MichalZalecki Maybe this is helpful:
https://jeremygayed.com/making-head-tag-management-thread-safe-with-react-head-323654170b45
https://github.com/tizmagik/react-head

I would help you out otherwise but I'm suuuper busy atm

@MichalZalecki MichalZalecki changed the title concurrent renders Critical: Concurrent renders Nov 4, 2018
jesstelford added a commit to jesstelford/react-portal-universal that referenced this issue Feb 18, 2019
jesstelford added a commit to jesstelford/react-portal-universal that referenced this issue Feb 18, 2019
@jesstelford jesstelford linked a pull request Feb 18, 2019 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants