single-spa / single-spa-react

Single-spa lifecycles helper for React applications
https://single-spa.js.org/docs/ecosystem-react.html
MIT License
227 stars 63 forks source link

Parcel mounts multiple times in the page #194

Closed fcano-ut closed 1 year ago

fcano-ut commented 1 year ago

Hello.

We're using single-spa-react to mount a React 17 Parcel into an Angular project.

We've noticed that when we include the update function in the exports, the Parcel renders twice in the page: one where it's supposed to be mounted, and once in the root HTML.

For example, if this is the Angular component tree:

<body>
  <wrapper-component>
    <sidebar/>
    <parcel .... />
  </wrapper-component>
</body>

...The component tree will end up like this when the parcel is replaced by a microfrontend:

<body>
  <wrapper-component>
    <sidebar/>
    <microfrontend .... />
  </wrapper-component>
  <microfrontend .... />
</body>

Not only that, but also, the second instance of the microfrontend never gets unmounted from the page, so if you navigate back and forth to the view that has the Parcel component, you'll end up with several duplicated instances of the microfrontend in the body of the page, like so:

<body>
  <wrapper-component>
    <sidebar/>
    <microfrontend .... />
  </wrapper-component>
  <microfrontend .... />
  <microfrontend .... />
  <microfrontend .... />
  <microfrontend .... />
  ...
</body>

We managed to solve this by removing the update export from the result of singleSpaReact:

  import * as React from 'react';
  import ReactDOM from 'react-dom';
  import singleSpaReact from 'single-spa-react';
  import App from './app';

- export const reactLifecycles = singleSpaReact({
+ export const { mount, unmount, bootstrap } = singleSpaReact({
    React,
    ReactDOM,
    rootComponent: App,
    errorBoundary: (err, info, props) => (...),
  });

Using singleSpaReact without the import is also the recommended way to use it according to the documentation.

But paradoxically, removing the update method from the exports is also the cause of issue #87.

I see that the change in the documentation was made alongside the release that gives support to React 18 (this PR: https://github.com/single-spa/single-spa-react/pull/148), so I wonder if it's possible that something broke along the way.

We'd like to be able to update our parcel and not having it render twice in the page.

I was trying to figure out what's happening under the hood, and I realize that there's a method that returns null and doesn't store the renderResult correctly when using ReactDOM render method (<=v17), it's here: https://github.com/single-spa/single-spa-react/blob/main/src/single-spa-react.js#L245-L249

I wonder if not storing the render result is what's causing these issues. I'm not sure if there's a place where integration tests are being run in this project, or if I should boot up a test app from scratch to reproduce the issue. Let me know what could be more helpful. Thanks!

MilanKovacic commented 1 year ago

Hi, thank you for reporting the issue. It would help a lot if you could try to reproduce the issue in a minimal repository that we could take a look at.

fcano-ut commented 1 year ago

I'll work on a reproduction repo tomorrow!

fcano-ut commented 1 year ago

I could not reproduce this issue in a clean environment, unfortunately 😞 I'll close it for now

fcano-ut commented 1 year ago

Ok, after some more fiddling I was able to reproduce the issue.

To be more truthful to how our app works, I used this setup:

I'm not 100% sure if this issue is in this package or in the angular package right now, but I'll leave the details of what I know and a repository to test.

This is the repository to reproduce the issue: https://github.com/fcano-ut/single-spa-test

The key to reproduce is to first mount the parcel, and then update any of it's props. I'm using code like this in the Angular host:

export class AppComponent {
  title = 'angular-host';

  public customProps: any = {
    message: "I'm passing a prop on mount!"
  };

// ...
  ngOnInit() {
    // ...
    window.setTimeout(() => {
      this.customProps = {
        message: "I'm updating props!",
      };
    }, 100);

https://github.com/fcano-ut/single-spa-test/blob/main/packages/angular-host/src/app/app.component.ts#L13-L29

The react microfrontend simply prints the message like so: {props.name} is mounted! and it's saying: {props.message}

If we load this in the browser, it's clear that the microfrontend loads twice, the second time at the end of the page:

Screenshot 2023-10-18 at 09 47 23

If we stop exporting the update function on the microfrontend side (removing it from here), it loads only where it should, but it doesn't receive updates on props.

Screenshot 2023-10-18 at 09 48 55

Hope this is helpful.

fcano-ut commented 1 year ago

Sorry, I opened by mistake, let me just be 100% this issue is not on the angular package before reopening! I will try and have an example using React as host and see if the issue still happens

UPDATE: Confirmed this is an issue in the Parcel component of single-spa-angular 😓

Sorry about the confusion here