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

[react-intl] Context exposed on window is causing issues within a microfrontend infrastructure #4118

Closed
voodoocreation opened this issue Jun 1, 2023 · 7 comments
Labels

Comments

@voodoocreation
Copy link
Contributor

Which package?

react-intl, version >= 6.3.0.

Describe the bug

In our organisation, we use a combination of single-page apps (SPAs) and microfrontends (MFEs), each of these projects is completely independent from each other and have their own dependencies, and are loaded via static assets which are compiled with TypeScript via Webpack. The problem with this particular code is that it forces all of them to use the react-intl context generated by the first thing to load (the SPA generally), which may be on a completely different version of react-intl to any of the MFEs that are loaded on the page, which means if these versions use an incompatible context, there's no way to engineer around that, as no alternative is provided within react-intl to avoid using window.__REACT_INTL_CONTEXT__, which leads to errors being thrown when some expected values within the react-intl context aren't defined, due to it being generated by a different version of react-intl.

It would be very useful to allow some way to override this behaviour, to keep the context strictly to the scope of each bundle without it depending on global values on window

To Reproduce

You would need to have a SPA using one version and then have it load an MFE that's on a different version that expects a different context to be present.

Codesandbox URL

Unfortunately the codebases are private within my organisation, so cannot be shared.

Expected behavior

I would expect each set of bundled code to use its own react-intl context, rather than inheriting whatever is on window.

Screenshots

If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

It's not device or browser specific, but rather an issue with relying on globally-exposed values when you can't guarantee consistency within all infrastructures.

@longlho
Copy link
Member

longlho commented Jun 1, 2023

I'd recommend not using different versions of react-intl. The global context was created to exactly combat MFE issues where imports don't resolve to the same Provider, thus causing mismatched Provider issue.
Another solution is you can use patch-package to remove the global context, but we won't remove it from this library.

@longlho longlho closed this as completed Jun 1, 2023
@voodoocreation
Copy link
Contributor Author

voodoocreation commented Jun 1, 2023

That's not possible in our case, we have teams all over the world who own their own codebases and deploy independently from each other, with different maintenance schedules, so it's impossible to ever guarantee that all codebases have the same version of react-intl installed.

We don't have the mismatched provider issue because the provider is specific to each bundle - we have no issues with module resolution because our infrastructure operates differently to the scenario you described, whereas window.__REACT_INTL_CONTEXT__ is different because the first set of JS to execute sets it. So in trying to solve one issue, a different issue was created on the opposite side.

There should be some way of opting out of this shared window context so that it can work for both types of architecture, rather than simply saying "don't use different versions" which isn't possible for organisations such as the one I belong to, with this sort of infrastructure.

@longlho
Copy link
Member

longlho commented Jun 1, 2023

I'd recommend you use patch-package to remove it then.

@voodoocreation
Copy link
Contributor Author

I just want to reiterate that within a large organisation, it's not particularly scalable or sustainable to have hundreds/thousands of repos all patch this problem using patch-package. It would be much more sustainable to make this behaviour optional in one place in the source package. Theoretically, it wouldn't be that hard to make it a prop of the provider to set in the upper scope and then conditionally use window or not based on that value.

I do understand your reasons for implementing it in this way, but it doesn't account for every possible infrastructure that your package is used within. It's sensible to make it the default behaviour for sure, but to not allow it to be opted out of creates major problems for other use cases.

@longlho
Copy link
Member

longlho commented Jun 2, 2023

I'm open to making this optional if there's a PR.

@voodoocreation
Copy link
Contributor Author

Thank you - I'll see what I can do 🙏

@voodoocreation
Copy link
Contributor Author

Have opened a very simple PR (#4125). I tried to avoid introducing any duplication or fragmentation and kept it very simple so that it won't impact future changes or the way that anything else is wired up and simply allowing it to work the way it did prior to 6.3.0 when explicitly desired.

longlho pushed a commit that referenced this issue Jun 6, 2023
…4125)

Co-authored-by: Raice Hannay <raice.hannay@xero.com>
unional pushed a commit to unional/formatjs that referenced this issue Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants