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.

Sign up for GitHub

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

Jump to bottom

[URGENT] react-toast is injected to DOM by default causes multiple web app using Style UI 2.x to fail on load #210

Closed
joyfulelement opened this issue Aug 7, 2020 · 5 comments
Closed

[URGENT] react-toast is injected to DOM by default causes multiple web app using Style UI 2.x to fail on load #210

joyfulelement opened this issue Aug 7, 2020 · 5 comments
Labels
🐛 bug Something isn't working

Comments

@joyfulelement
Copy link

joyfulelement commented Aug 7, 2020 •
edited

Business Impact

  • An on going issue on all deployed environments (dev, stage, prod)
  • Blocking teams from adopting Styled UI 2.x

Problem

Root Cause

  1. react-toast component is injected to DOM by default when using Styled UI even if web application doesn't use Toast component.
<html class="ext-strict">
  <head>
    ...
  </head>
  <body>
    <header id="top-header">...</header>
    <main id="app" class="app">...</main>
    <div id="react-toast" class="Toaster">  <!-- <==== injected to DOM by default when using Styled UI -->
      ...
    </div>
  </body>
</html>
  1. The underlying 3rd party library toast-note used by Toast component has the assumption that Toast component is only used by a single web app with a fixed ID: https://github.com/bmcmahen/toasted-notes/blob/master/src/Alert/Toast.tsx#L27

Effect & Impact

  • This becomes an issue if we dynamically mount and unmount microfrontend web apps (e.g. with single-spa) that uses Styled UI, as single-spa is the container for it.

  • <body> element in DOM is littered with instance of react-toast element injected from last microfrontend web apps using Styled UI

Reproduced Steps

The reproduced steps are very simple and captured on the internal JIRA, since it's causing an incident and impacting our production environment, those details are only shared within the JIRA case. Please consult @ultralabed for detail.

Proposed Solutions

Workaround

  • Change the default behaviour of Styled UI so it does not inject Toast component by default.
    However, without the proper fix (as proposed below) the of root cause, the problem will against resurface if multiple web app begin using Toast component.

Proper Fix on the Root Cause

  1. Fork the 3rd party library toast-note of that toaster component and modify the code to be able to pass in an ID where to inject the toast component.
    OR
  2. Find a new (and better) 3rd party component that only adds the notification component to the DOM on demand and removes itself from DOM immediately afterwards.
@ultralabed ultralabed pinned this issue Aug 7, 2020
@ultralabed ultralabed added the 🐛 bug Something isn't working label Aug 7, 2020
@roth1002
Copy link
Member

Try

import theme from '@trendmicro/react-styled-ui/theme';

@joyfulelement
Copy link
Author

joyfulelement commented Aug 12, 2020 •
edited

Try

import theme from '@trendmicro/react-styled-ui/theme';

Thanks @roth1002. Our team have tried this but didn't work. I've copied the email reply from the developer about this in case you didn't receive it:

I tried the solution that you suggested to use exact import for the theme. But still getting same error. 
I tried adding styled-guide to both Cloud One Header and Landing page apps and do imports as suggested.
 
I also tried using the recommended setup (https://trendmicro-frontend.github.io/styled-ui/getting-started) in Landing which doesn’t require any explicit import for theme. 
Even in that case, same error is thrown.

@gsaran
Copy link

gsaran commented Aug 12, 2020 •
edited

As an update to the previous comment by @joyfulelement, I further extended the exact import to be used for each component and not just theme. For instance, an application using multiple components like Box, Menu, MenuList etc. needs to do exact import for each component:

import Box from '@trendmicro/react-styled-ui/Box'; 
import Menu from '@trendmicro/react-styled-ui/Menu'; 
import MenuList from '@trendmicro/react-styled-ui/Menu/MenuList';

Although, it makes code bit clumsy but it's a temporary fix for now to avoid injection of Toast element into the DOM.

@cheton
Copy link
Member

cheton commented Sep 7, 2020

Resolved in 2a90d3f

It will be available in v0.8 or a later release.

@cheton
Copy link
Member

cheton commented Sep 11, 2020

Fixed in 0.8.0

@cheton cheton closed this as completed Sep 11, 2020
@cheton cheton unpinned this issue Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🐛 bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@cheton @joyfulelement @roth1002 @ultralabed @gsaran