gsoft-inc / sg-orbit

The design system for ShareGate web apps.
https://orbit.sharegate.design
Apache License 2.0
101 stars 37 forks source link

❗ Improve encapsulation of ThemeVars #1088

Open tjosepo opened 1 year ago

tjosepo commented 1 year ago

Description

Currently, the createThemeVars() function appends CSS variables directly in the document's head.

If multiple version of Orbit are loaded on the same page and call createThemeVars(), the variables can be overriden.

Ideally, each <ThemeProvider /> should be able to have its own set of CSS variables that cannot be overriden by another theme provider.

Otherwise, having more than one version of Orbit on a page can be brittle, which limits what can be done with micro-frontends.

patricklafrance commented 1 year ago

Hey @tjosepo!

Does this currently block your team?

I doubt it’s a good idea to load multiple versions of Orbit even in a micro-frontends application.

The host application should be in charge of loading the design system and the modules should use whichever version is available.

To bump the version, update the host application and it will replicates everywhere.

The downside thought is that Orbit will have to minimize breaking changes or at least offer temporary backward support to prevent the modules from breaking.

The upside is that the application won’t load Orbit bundles many times and by forcing every module to be in the same version it ensure the UI stay consistent.

What use cases do you have in mind?

tjosepo commented 1 year ago

Does this currently block your team?

It's not a blocker, but having to synchronize dependencies and releases between micro-frontend is definitely a pain point.

React already supports multiple versions on a single page, and I think a similar thing happening to Orbit could solve some of our challenges.

I doubt it’s a good idea to load multiple versions of Orbit even in a micro-frontends application.

I agree that it's not ideal, but I think we could benefit from being able to do so when dealing with breaking changes.

The nice thing about Module Federation is that it's smart enough to reuse the same dependencies if they match versions, and load them separately if they mismatch versions.

In the majority of cases, Orbit would be the same version across micro-frontends, and would only be downloaded once.

The downside thought is that Orbit will have to minimize breaking changes or at least offer temporary backward support to prevent the modules from breaking.

100% agree.

What use cases do you have in mind?

Recently, there were breaking changes to some tokens. Micro-frontend apps had to be backward compatible, otherwise, some tokens and background would be invalid. 😔

Not updating the micro-frontend first prevented others from updating the version of Orbit on the host.

patricklafrance commented 1 year ago

Thanks for your reply!

React already supports multiple versions on a single page

I don't think I understand what you mean here. From my experience, having 2 distinct versions of React on the same page will result in the following error:

hooks can only be called inside the body of a function component

The nice thing about Module Federation is that it's smart enough to reuse the same dependencies if they match versions, and load them separately if they mismatch versions.

I used to think that Module Federation is smart enough to reuse the same dependency if they match versions, but I don't think anymore that it can do that. I might be wrong thought.

I think that to reuse the same dependencies, the dependency has to be explicitly specified as a shared singleton dependency.

Let me explain how I think it's working:

At build time

  1. For each module, Webpack go through the configured exposes entry in the ModuleFederationPlugin configuration. Those exposes entry are considered as entry points by Webpack and it create a bundle file for each of them. Those bundle files include the actual module code and the "non shared" dependencies that they depends on.
  2. Dependencies that are defined as shared entry in the ModuleFederationPlugin configuration are bundled in standalone files instead.
  3. The same is done for the host application.

At runtime

  1. When the host application encouter a remote import, if it's not an explicitly defined "shared dependency", the module bundle matching that remote import is downloaded by the host application
  2. Since at build time, Webpack couldn't know which "non shared" dependencies used by the module are also used by other parts of the application, Webpack included every "non shared" dependency it encoutered in the "exposes" entry bundle.
  3. Therefore, since those "non shared" dependencies were included in the bundle file that the application download in step 1. the cost as already been paid, even if somehow ModuleFederationPlugin ends doing smart things to dedupe that dependency once it's loaded.
patricklafrance commented 1 year ago

Hey @tjosepo

I was thinking about this issue and 2 questions comes to my mind, maybe you also thought about those and have solutions?

tjosepo commented 1 year ago

createThemeVars() is always called in an index file before rendering the React app, where would you call it for a module?

I'm wondering if we could drop it in the createThemeVars() inside the ThemeProvider? It always felt a bit wierd DX wise to have to load two different things for the Orbit theme to work properly.

One problem I can already think of with this approach is that I think some component (i.e. modals) are wrapped inside their own ThemeProvider, but we'd only want to redeclare the theme variables if they haven't been set before.

It's one thing to scope the theme CSS variables to a theme provider but what about the components CSS files? They also must be scoped for this to work.

Scoping CSS would be useful if we wanted to load multiple versions of Orbit on the same page, but most CSS rules can already be scoped inside a Web Component ShadowRoot.

One of the few things that can traverse a ShadowRoot boundary is CSS variables.

Using ShadowRoot definitely has its downsides. React doesn't really support them. You need to manually create a new React root inside the ShadowRoot. This means that providers need to be redeclared and all providers that don't rely on an external store and don't useuseSyncExternalStore won't be synchronized across roots... Less than ideal, but it's something to consider.

patricklafrance commented 1 year ago

Thanks for sharing your thought.

createThemeVars

Moving the creation of the vars into the ThemeProvider will require to change it's purpose and it's usage. Currently, it's possible to to nested multiple ThemeProvider.

I agree thought that it's an interesting option to consider to support multiple version of the DS in a federated apps.

ShadowRoot

Let me know if you find something interesting with this lead. Pretty sure I read somewhere that React is working on Web Component support.