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

usePortals is buggy #2226

Open
SebastianStehle opened this issue Dec 18, 2023 · 0 comments
Open

usePortals is buggy #2226

SebastianStehle opened this issue Dec 18, 2023 · 0 comments
Labels
status: sponsored 💖 All issues and pull requests which have been requested from or created by sponsors. type: bug 🪲 Something isn't working

Comments

@SebastianStehle
Copy link
Sponsor

SebastianStehle commented Dec 18, 2023

Summary

Hi,

I am using remirror in an angular app and in the migration from angular 16 to 17 my portals stopped working.

It is basically a race condition in the current code, that I can only reproduce within angular and I don't know why exactly. But I can show in the code that it might happen.

Steps to reproduce

As I said I cannot reproduce it really in a small case, but I can show you at the code how it works:

The following code is from the portal container and usePortals hook. I have simplified it a little bit. In my comments you can see in which order the code runs in my application.

export function usePortals() {
  const [portals, setPortals] = useState(() =>  {
      // RUNS FIRST. Portal Count in Hook = 0
      return Array.from(portalContainer.portals.entries()));
  }

  useEffect(
    () =>
      
      // RUNS THIRD: Portal Count in Hook = 0, Portal Count in Container = 1.
      // HERE A CALL to setPortals is missing.
      portalContainer.on((portalMap) => {
        setPortals(Array.from(portalMap.entries()));
      }),
    [portalContainer],
  );
  
  // Is this not useless?
  return useMemo(() => portals, [portals]);
}

export class PortalContainer {
   private update() {
      // RUNS SECOND: Portal Count in Container = 1,
       this.#events.emit('update', this.portals);
  }
}

Expected results

Actual results

Possible Solution

Add the following call to useEffect

setPortals(Array.from(portalContainer.portals.entries()));

I have applied that with https://www.npmjs.com/package/patch-package and it works as expected.

Screenshot(s)

@SebastianStehle SebastianStehle added the type: bug 🪲 Something isn't working label Dec 18, 2023
@github-actions github-actions bot added the status: sponsored 💖 All issues and pull requests which have been requested from or created by sponsors. label Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: sponsored 💖 All issues and pull requests which have been requested from or created by sponsors. type: bug 🪲 Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant