Topics

Theo's Typesafe Cult•8mo ago

React Portal renders the element passed as children 2 times

Any idea how to get around this? Idea to troubleshoot? I tried reproducing it on stackblitz, using the same state management library I'm using in the project (zustand) and I wasn't able to. There it renders once, as expected. Came across this while learning E2E testing. Here's my Portal:

"use client"

import { createPortal } from 'react-dom';
import { ReactNode, useEffect, useState } from 'react';
import React from 'react';

type PortalProps = {
  children: ReactNode;
};

export default function Portal({ children }: PortalProps) {
  const [modalElem, setModalElem] = useState<Element | null>(
    typeof document !== 'undefined' ? document.querySelector('#popup-root') : null
  );

  useEffect(() => {
    setModalElem(document.querySelector('#popup-root'));
  }, []);

  if (!modalElem) {
    return null;
  }

  return createPortal(<>{children}</>, modalElem)
}

"use client"

import { createPortal } from 'react-dom';
import { ReactNode, useEffect, useState } from 'react';
import React from 'react';

type PortalProps = {
  children: ReactNode;
};

export default function Portal({ children }: PortalProps) {
  const [modalElem, setModalElem] = useState<Element | null>(
    typeof document !== 'undefined' ? document.querySelector('#popup-root') : null
  );

  useEffect(() => {
    setModalElem(document.querySelector('#popup-root'));
  }, []);

  if (!modalElem) {
    return null;
  }

  return createPortal(<>{children}</>, modalElem)
}

the div with #popup-root is in my layout.tsx component and the Portal is conditionally rendered using a flag from a page.tsx.

No description

Solution:

that's why I'd take this one step further and take the hamburger menu out of the NavBar and make it a sibling component instead... so it's more clear that no one component "owns" it but any other component can open/close it

Jump to solution

47 Replies

michaeldrotar•8mo ago

did you try passing a key? they generally de-dupe things - createPortal(children, node, key)

beroerOP•8mo ago

i have now, but the behavior is the same. it can be any arbitrary string value serving the purpose to differentiate between portals right?

michaeldrotar•8mo ago

yup Seems like such an odd issue. The key should be optional and the forced re-render on dev shouldn't be an issue but I'm thinking there must be some bad assumption definitely don't have <Portals /> in two places?

beroerOP•8mo ago

i'm using the same portal component i showed to render its children a bunch of times, but from where the hamburger-menu should appear, it's only 1 condition and the portal with its children: excerpt for two portals from the same component, NavBarMobile:

        {showMobileSearch && (
          <Portal key="mobile search">
            <MobileSearch />
          </Portal>
        )}

        {showHamburgerMenu && <Portal key="hamburger"><HamburgerMenu /></Portal>}

        {showMobileSearch && (
          <Portal key="mobile search">
            <MobileSearch />
          </Portal>
        )}

        {showHamburgerMenu && <Portal key="hamburger"><HamburgerMenu /></Portal>}

i'm having the issue when showHamburgerMenu or any other flag becomes true. because inside HamburgerMenu i'm specifying a testid, which exists only in HamburgerMenu and nowhere else, and i see the same testid 2 times within the #popup-root div, into which the Portal ports its children. (within the DOM in devtools, once i click and showHamburgerMenu becomes true)

michaeldrotar•8mo ago

Are you seeing the behavior running it in dev or in the E2E test? (or both?) And do you think Zustand has anything to do with it? I don't see it involved in the code you shared, just asking since you mentioned it.

beroerOP•8mo ago

i'm seeing the behavior in both dev and E2E. i don't know if zustand has anything to do with it, phind suggested something about third party state when i asked about it, so when trying to reproduce the issue i tried both react state and zustand state as the flag and there both worked with the Portal rendering its children only once. showMobileSearch and showHamburgerMenu are state flags managed by zustand, i didn't include that, but here's how i'm calling it:

  const { showHamburgerMenu, setShowHamburgerMenu } = useNavigationStore()

  const { showHamburgerMenu, setShowHamburgerMenu } = useNavigationStore()

and the definition:

import { create } from 'zustand'

type TNavigation = {
  showHamburgerMenu: boolean
  setShowHamburgerMenu: (value: boolean) => void
}

export const useNavigationStore = create<TNavigation>((set) => ({
  showHamburgerMenu: false,
  setShowHamburgerMenu: (value) => set({ showHamburgerMenu: value })
}))

import { create } from 'zustand'

type TNavigation = {
  showHamburgerMenu: boolean
  setShowHamburgerMenu: (value: boolean) => void
}

export const useNavigationStore = create<TNavigation>((set) => ({
  showHamburgerMenu: false,
  setShowHamburgerMenu: (value) => set({ showHamburgerMenu: value })
}))

michaeldrotar•8mo ago

You find the most bizarre issues 😆

beroerOP•8mo ago

:thinkies:

michaeldrotar•8mo ago

I'm trying to think of how it could even theoretically happen like two #portal-root elements, but even that'd be fine cause querySelector will only take one of them Or if the portal target element changed and somehow the content got left in the old element but also duplicated to the new one.... but that seems to be fine too (I made a "foo" button that swaps portals) I can only reproduce it from actually having it twice in my code https://stackblitz.com/edit/nextjs-tyoknk?file=pages%2Findex.js,pages%2FPortal.jsx,pages%2F_app.js

beroerOP•8mo ago

the path of testing hasn't been the most... intuitive one i guess 😄 there's a single popup-root, i see that in the dom. there's only 1 portal target element i'm using all throughout the website

michaeldrotar•8mo ago

what I mean is how you showed using one for mobile search and one for the hamburger menu if you had another one somewhere else (like one in _App and one in some page) or if the component using them was used in multiple places.. like a <Navbar /> in 2 nested layouts that both used <Portal><Thing /></Portal>

beroerOP•8mo ago

i'm not sure if i follow, but if i understand correctly, you're saying that if there's 2 instances of the same parent component in which the Portal code resides in, then it could be rendered 2 times by one click... ? i have one navbar for mobile, i have another hamburger icon in another place for tablet view, for which the HamburgerMenu is relevant and there's no duplication as far as i can tell. (btw it's a nextjs project) i'm not sure if i understood correctly though. the common denominator seems to be the Portal code. because the duplication happens anywhere i'm using the Portal at. regardless of the flag, or the library i'm using for the flag

michaeldrotar•8mo ago

I think we're saying the same thing, yeah like in my demo I have <Foo /> in _App and index and Foo uses <Portal>...</Portal> so the foo button is rendered twice https://stackblitz.com/edit/nextjs-tyoknk?file=pages%2Findex.js,pages%2FPortal.jsx,pages%2F_app.js,pages%2FFoo.js here's a test... add this to hamburger menu:

const [count, setCount] = useState(0)
...
return <>
  ...
  <button onClick={() => setCount(x => x + 1)}>{count}</button>
</>

const [count, setCount] = useState(0)
...
return <>
  ...
  <button onClick={() => setCount(x => x + 1)}>{count}</button>
</>

Does clicking one button increment both or just itself?

beroerOP•8mo ago

just itself.

No description

beroerOP•8mo ago

HamburgerMenu is positioned absolutely so they're on top of one another.

michaeldrotar•8mo ago

yeah can break that temporarily to see more clearly if it's just itself then it's 2 instances with their own state rather than the HTML rendered twice

beroerOP•8mo ago

i wonder where am i doing something wrong to cause this

michaeldrotar•8mo ago

have the code somewhere public that I could flip through it?

beroerOP•8mo ago

yes, one sec i'll commit some changes and push it.

beroerOP•8mo ago

ok i just pushed. https://github.com/waikoo/thrifty

GitHub

GitHub - waikoo/thrifty: A mock online thrift-shop

A mock online thrift-shop. Contribute to waikoo/thrifty development by creating an account on GitHub.

beroerOP•8mo ago

i very much appreciate it i wrote the readme without actually checking, i'm not even sure you need a supabase account to get it working even, i guess i'll find out now. i can give you my credentials if there's any issues so you don't have to make one

michaeldrotar•8mo ago

not running it just skimming through does every portal get doubled?

beroerOP•8mo ago

as far as i can tell, yes. i've checked the two Portals in: NavBarMobile and both got doubled. didn't check others.

beroerOP•8mo ago

if https://thrifty-seven.vercel.app is blank, try https://thrifty-seven.vercel.app/en/women if you wanna see the live version

An e-commerce store for second-hand clothing

michaeldrotar•8mo ago

one thing that jumps out is that HamburgerMenu is in two places: NavBar and NavBarMobile and while NavBar has hidden sm:block, that css won't apply to the HamburgerMenu since it's sent to a portal that's removed from those styles (also it's still in the DOM, hidden or not) so there is potential to have 2 of them here I think that's it cause there's only a single hamburger menu on the cart route - which isn't part of that [gender]/page path

beroerOP•8mo ago

even though in tablet view (when NavBar applies) (from sm:+) the hamburger button is on the top of the viewport (next to the search) and in mobile view (when NavBarMobile applies) it's on the bottom of the viewport?

michaeldrotar•8mo ago

yeah the visual position of them doesn't matter cause the HamburgerMenu is sent through a Portal so if any page, like your [gender]/page one, has both NavBar and NavBarMobile then you'll have 2 HamburgerMenu instances

beroerOP•8mo ago

so what could i do to have only one?

michaeldrotar•8mo ago

Zustand is a factor here because if you were using local state, then { showHamburgerMenu && ... } would be a different state variable for each navbar* component and only one would render - but using zustand for that state means they share it so "showing" one shows both

beroerOP•8mo ago

i see so using local state would be one option

michaeldrotar•8mo ago

I'd probably make the hamburger menu it's own top-level component.. so navbar and the menu are siblings and zustand facilitates communication across them you can see the fix more quickly simply removing the second instance from NavBarMobile - since NavBar already includes it on every page Zustand will still let the button open that instance of it ie delete line 61 on NavBarMobile

beroerOP•8mo ago

wow i removed it and indeed there's only one now. i'm having a hard time understanding how come it still pops out from NavBarMobile even though it's commented out.

michaeldrotar•8mo ago

what do you mean it's commented out?

beroerOP•8mo ago

instead of removing the line i commented it out. from NavBarMobile

michaeldrotar•8mo ago

{/* { showHamburgerMenu ... } */}

{/* { showHamburgerMenu ... } */}

like this? that should be equivalent to deleting the line

beroerOP•8mo ago

yes

michaeldrotar•8mo ago

oh, you're saying how come it works when it's removed

beroerOP•8mo ago

oh i think the point is that it doesn't matter from WHERE it's coming from the point is WHERE IT'S GOING TO yes i made a step forward but it's still a bit hazy, i'm trying to follow. since zustand is global state, and i had the HamburgerMenu instanced 2 times, once in NavBarMobile and once in NavBar, but using the same global zustand flag, once that flag flipped by either trigger, it instanced HamburgerMenu twice, right?

michaeldrotar•8mo ago

so normally when using useState() - each time it's called is independent whether it's across different components or multiple times in the same component

const [a, setA] = useState('')
const [b, setB] = useState('')

const [a, setA] = useState('')
const [b, setB] = useState('')

These are separate and independent, right? But when you use Zustand, the problem it's solving is how to share state across components.

const nav = useNavigationStore()

const nav = useNavigationStore()

If 50 different components all do this, they'll all have the same nav object Does that much make sense? Yeah that sounds like you got it

beroerOP•8mo ago

oh great, thanks for the explanation

Solution

michaeldrotar•8mo ago

that's why I'd take this one step further and take the hamburger menu out of the NavBar and make it a sibling component instead... so it's more clear that no one component "owns" it but any other component can open/close it

michaeldrotar•8mo ago

I'd probably also put <Portal> inside the hamburger menu so it's more self-contained and use some doc comments to explain it should only exist once

beroerOP•8mo ago

but if the hamburgermenu would be a top-level component, in the page... its visibility could still be modified from within itself by the zustand store, right? so i wouldn't have to make my whole page a client component. you mean like, within HamburgerMenu:

return showHamburgerMenu && <Portal>
    ... {/*HamburgerMenu markup*/}
</Portal>

return showHamburgerMenu && <Portal>
    ... {/*HamburgerMenu markup*/}
</Portal>

?

michaeldrotar•8mo ago

yup like that that'll also make it easier if you want to animate it opening in the future, you won't have to go around finding all the { showHamburgerMenu && ... } things, just update the one spot I think it'd go in (client)/layout so all your client pages have access to it just like how Header there is a client component but other parts can still be a server component

beroerOP•8mo ago

all right. i just 'figured' Portals 'out' on the go once, and kept using it like that ever since, i wasn't aware of all these different ways to structure it or what could lead to what when used like, thanks for the update. 😄 i'll still need to try hard to warp my head around this. i feel like you made me zoom out and see more. 😄

michaeldrotar•8mo ago

lol glad I could help, can be easy to get lost in the weeds and need a rubber duck from time to time

beroerOP•8mo ago

but if you meant yourself for a rubber duck, you're at least one that's sitting on its own brains-level rubber duck! i really appreciate all the brains.

Theo's Typesafe Cult Join

21KMembers

View on Discord

Want results from more Discord servers?

Add your server