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

Some non-hook function results are not preserved after conditional OutPortal #39

Open
paulsohn opened this issue Sep 17, 2022 · 2 comments

Comments

@paulsohn
Copy link

paulsohn commented Sep 17, 2022

I would like to

  • transfer components which render results depend on nondeterministic, direct function call(such as Math.random() or Date.now()) and
  • fix the results before and after transfer, without storing them internally
  • OutPortal for the components are conditional

And for these purposes, it seems like this package doesn't do what I expected.


Here's the MWE example: CodeSandbox Link

//Box.tsx
function Box({ txt }: BoxProps) {
  return (
    <div style={{ border: "1px solid red" }}>
      <strong>{txt}</strong>
      <br />
      {Date.now()}
    </div>
  );
}
export const MemoBox = memo(Box);
//App.tsx
import { MemoBox } from "./Box";

function App() {
  const [show, setShow] = useState(true);
  const node1 = useMemo(createHtmlPortalNode, []);
  const node2 = useMemo(createHtmlPortalNode, []);
  const toggle = useCallback(() => { setShow((show) => !show); }, []);

  return (
    <>
      <InPortal node={node1}> <MemoBox txt="111" /> </InPortal>
      <InPortal node={node2}> <MemoBox txt="222" /> </InPortal>
      <button onClick={toggle}>Click me!</button>

      <div>
        {show && <OutPortal node={node1} />}
        {show && <OutPortal node={node2} />}
      </div>
      <div>
        {!show && <OutPortal node={node1} />}
      </div>
    </>
  );
}

Every time I toggle, Box "111" alters its position and Box "222" repeats hiding and showing. Now let's focus on the timestamp.
Box "111" prints with the fixed timestamp, because every render exposes either OutPortal node1. This is fine.
Box "222" however, the timestamp has changed on every show, and this behavior is not quite intuitive when the package description says:

Rendering to zero OutPortals is fine: the node will be rendered as normal, using just the props provided inside the InPortal definition, and will continue to happily exist but just not appear in the DOM anywhere.

rather, it looks like when OutPortal node2 is absent, the detached node2 has been garbage collected and previous timestamp is gone forever.

I'm aware that below solutions will fix Box "222" timestamp as well, so that my purposes are satisfied:

  • modifying the Box implementation (namely using useEffect and useState) to store the timestamp right after mounting
  • or even simpler, insert !show && OutPortal node2 as well but wrapped inside a display:none div.

yet I'm curious why did the timestamp change when the component is supposed to be memoized - given that without these portals React.memo() will do its work and fix the whole results.

@pimterry
Copy link
Member

Huh, interesting, thanks for reporting this @paulsohn!

it looks like when OutPortal node2 is absent, the detached node2 has been garbage collected and previous timestamp is gone forever.

I don't think that's correct. This is showing that React is doing a re-render for this component, but that doesn't mean the DOM nodes are otherwise affected, they're relatively independent processes. You can see that the DOM nodes aren't garbage collected when removing from the page by testing something where the DOM itself has state (rather than React), e.g. remove and re-add a playing <video> element or a form containing input.

That doesn't help though, because I'm not sure why React is re-rendering to update the value here! That suggests that it thinks the props for this MemoBox have been updated during this process for some reason... Any ideas?

I think that React.memo() is only a shallow comparison over all props, so I suspect this is happening because of some prop update we're doing that means that a prop has a top-level change (e.g. a cloned array or object) even though the internal values haven't changed. Not sure where exactly though.

You might be able to find out more digging into this with the React devtools, which will tell you exactly why each component re-renders.

If the above is correct, you can also probably fix it by passing a 2nd argument to React.memo(), to define exactly what you're memoizing against. In this case, instead of a shallow comparison of all props, what you really want is something like (oldProps, newProps) => oldProps.txt === newProps.txt (i.e. props only count as different if txt changes). If you test that, do let me know if it works, that'd be a strong sign that some props equivalent-but-not-technically-equal update is the cause.

@paulsohn
Copy link
Author

Thanks for your comment!
Hotfixing with export const MemoBox = memo(Box, isEqual) didn't helped me though - the props are already shallow enough.

BTW I'm currently working with my toy project using this awesome package and need to implement this kind of pattern - and that project doesn't suffer from this issue. This makes me more curious than ever... hopefully I can get back after I figured out what's the difference.

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

No branches or pull requests

2 participants