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

getModalizer backward incompatibility issue #290

Open
KagamiChan opened this issue Jul 10, 2023 · 7 comments
Open

getModalizer backward incompatibility issue #290

KagamiChan opened this issue Jul 10, 2023 · 7 comments

Comments

@KagamiChan
Copy link
Member

KagamiChan commented Jul 10, 2023

Our product is micro-frontend alike architecture, there's a host UI project and several guest UI projects that are all built and shipped differently, and all the dependencies are not shared. We discovered an issue during the upgrade of FluentUI v9, which is caused by different versions of tabster in host UI side and guest UI side.

A minimal repro could be found here: https://github.com/KagamiChan/multiple-fluentui-repro
I'm not using codesandbox because I need to use yarn resolutions to lock tabster version, please let me know if you prefer a codesandbox example.

tabster versions:
Host UI: 4.2.0
Guest UI: 4.6.0
main difference for this issue: tabster 4.3.0 introduces queue for initialization: #262

Steps to repro:

  • click the button to mount guest UI
  • error throws like below
image

what happened:

  1. host UI is mounted, and tabster core (v4.2.0) is initialized, while modalizer is not
  2. guest UI trying to mount, FluentUI tries to initialize modalizer, the getModalizer function and ModalizerAPI class is from guest UI (v4.6.0) so it calls tabster isntance's queueInit function
  3. tabster instance is initialized by host UI (v4.2.0) and does not have queueInit function, the exception throws

This issue is blocking us from the Fluent UI upgrade because we have to upgrade host UI everywhere but that would be difficult (our product is not served on web but is embedded), could you please help take a look and help us unblock?

@lesyk
Copy link

lesyk commented Jul 11, 2023

May you please clarify which Fluent version are you using?

@mshoho
Copy link
Member

mshoho commented Jul 11, 2023

If I understand things correctly, you want to use two versions simultaneously with the core of one version and the parts from another version. That wouldn't and shouldn't work. From the public API perspective Tabster should be compatible between the major versions. So, you should add a resolution to 4.6.0 from the host side first. There should be just one copy of Tabster in the bundle.

It doesn't concern just Tabster. If you need to use separate versions of anything simultaneously, you need to isolate the guests from the host using iframes.

@KagamiChan
Copy link
Member Author

@lesyk we're in the way to upgrade Fluent UI from 9.18.2 to 9.22.0. since tabster is not locked by Fluent UI (range using ^), so actually we're bumping tabster from 4.2.0 to 4.6.0

@KagamiChan
Copy link
Member Author

KagamiChan commented Jul 11, 2023

@mshoho Thanks for your comments. I've also tested the case that host version 4.6.0 + guest version 4.2.0, it would also throw because of the DummyInputObserver API change in #264, when opening a dialog in guest UI, updateOffsets would be called which is no longer provided in v4.6.0.

I understand that this micro-frontend pattern is less popular, but I think such requirement would exist. Let me try to explain more.

Our project is a WebUI embedded and shipped in Edge browser, so there's no way to populate 1 version immediately to all clients. We need to consider the time for users to check and download updates as well as release channels.

I firstly would like to suggest we skip iframe solution because iframe has its own problems in user experience and development, and we cannot quickly switch to iframes now given that we've spent much effort on current architecture since the era with Fluent UI v8.

Because both projects are developed, built and shipped separately, it is difficult to keep only 1 version of Fluent UI v9 especially v9 is under fast development with many new components created or unstable components promoted to stable. So as a result both host and guest UI would bundle its own versions of Fluent UI.

And because version propagation takes time, it's difficult to avoid the coexistence of different tabster versions. This should be temporary and both UI would be using the same version finally, but still I don't think UI should crash in this case.

We've done several updates of Fluent UI and this is the first time we have this incompat issue.

@mshoho
Copy link
Member

mshoho commented Jul 12, 2023

@KagamiChan it is not about the less popular pattern.

If you have a library that provides, let's say, class A and class B. You cannot mix instances class A from one version and class B from another version together. Even from the JavaScript perspective, instanceof (new A.v1) will never equal to instanceof (new A.v2) — they physically have different constructor. And they see different closures, different library-level global variables and so on.

And nobody will guarantee the compatibility of the private internal APIs between the versions. They are internal and private. Things will get even worse between the major versions where nobody will guarantee even the public API compatibility.

So, if the system uses A.v1 and B.v2 together at the same time, it's not a pattern issue, it is a system's design flaw. The fact that this is the first issue of that type you have encountered is likely just a sheer amount of luck. If you couldn't isolate things using iframes for some reason, your system should fundamentally address that flaw somehow differently. For example, you can try to avoid bundling the component library (and relative deps) inside both Host and Guest and use it as a separate dependency that is imported by Host and Guest at runtime from the same location. Or something else. The system shouldn't allow mixing parts of external deps from different versions in the same runtime.

@KagamiChan
Copy link
Member Author

KagamiChan commented Jul 12, 2023

Would it be helpful if tabster instance is more self-contained? for example, instead of calling an utility function getModalizer(tabster), could it be tabster.getModalizer()?

Thanks for you suggestion, but I'm afraid this often works well on web apps but not in our product's case. For us, host A and guest B are released separately and in parallel, and it depends on clients to check and download updates, adding up that there're multiple channels to release our projects to. This creates a dependency for B on A's version.

Well I understand that tabster might have to be a singleton for its design purpose, and while I need to discuss more within our team on your suggested solution, I'm planning to do a quick mitigation to patch the old tabster instance from guest UI side:

export const renderGuestUI = (root: HTMLElement) => {
  const tabster = getTabster(window);
  if (compare((tabster as any).core._version ?? '9.9.9', '4.3.0', '<')) {
    (tabster as any).core.queueInit = (callback: () => void) => setTimeout(callback, 0);
    (tabster as any).core.drainInitQueue = () => {};
    (tabster as any).core._dummyObserver.updatePositions = () => {};
  }
  ReactDOM.render(<App />, root);
};

This does not intend to fix the missing functionality, and something might not work properly. All its purpose is to prevent crashing the UI before the new version of host reaches the client side.

@mshoho
Copy link
Member

mshoho commented Jul 12, 2023

tabster.getModalizer means that it is not treeshakable and the modalizer code will get into the bundle regardless of being not used.

A better way to mitigate would be to call getModalizer(getTabster()) from your host side before the guest. That way the instance will be created already and when the guest will be calling getModalizer(), the instance created by the host will be reused.

But again, it seems to me that your system's design is asking for trouble and should be addressed from that point of view.

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

3 participants