Skip to content

Conversation

@jbroma
Copy link
Member

@jbroma jbroma commented Sep 20, 2023

Summary

Problem Statement:

In our current federated setup for React Native applications, a persistent issue emerged around the handling of shared dependencies, specifically with respect to deep imports from the 'react-native' library. These deep imports disrupted the efficient sharing of modules across federated boundaries. Furthermore, due to the challenges posed by these deep imports, our assetsLoader couldn't inline scaled assets in federated modules, leading to broken functionalities in some federated scenarios.

References:
StackOverflow discussion on deep imports with Module Federation
Webpack's resolveMatchedConfigs

Solution:

This PR updates the ModuleFederationPlugin introducing mechanisms to better handle these shared dependencies, and by extension, fixing the assetsLoader issue.

Changes Introduced:

  1. Enhanced Configuration Options:

    • Added reactNativeDeepImports?: boolean to ModuleFederationPluginConfig to allow the option to enable or disable adding React Native deep imports to shared dependencies.
  2. Deep Import Handling:

    • Incorporated logic to adjust shared dependencies to account for deep imports from 'react-native' (e.g., react-native/Libraries/Utilities/PixelRatio). This was achieved using an undocumented feature of webpack's ModuleFederationPlugin.
    • Added a mechanism to ensure deep imports from the react-native library and packages from the @react-native group are correctly shared and resolved.

With this setup, the PR description begins by setting the context and outlining the problem, then presents the solution and the steps taken to achieve it.

Test plan

  • Added unit tests for added functionality inside ModuleFederationPlugin.test.ts

@changeset-bot
Copy link

changeset-bot bot commented Sep 20, 2023

🦋 Changeset detected

Latest commit: 7c6808e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@callstack/repack Minor
testerapp Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@thymikee thymikee requested a review from RafikiTiki September 21, 2023 07:58
@asherLZR
Copy link

Do we know of any instances where we might want to toggle reactNativeDeepImports: false? It seems RN internals like react-native/Libraries/Image/AssetRegistry and react-native/Libraries/Utilities/codegenNativeComponent which handle local images and native components rely on it always being shared/singleton.

@jbroma
Copy link
Member Author

jbroma commented Sep 22, 2023

Do we know of any instances where we might want to toggle reactNativeDeepImports: false? It seems RN internals like react-native/Libraries/Image/AssetRegistry and react-native/Libraries/Utilities/codegenNativeComponent which handle local images and native components rely on it always being shared/singleton.

While I agree with what you said, I'm including this as an option to opt out of this solution as from previous experiences, there might be a use-case where that's going to cause issues.

Copy link
Collaborator

@RafikiTiki RafikiTiki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great investigation and RN-specific resolution with the undocumented webpack API.

@jbroma jbroma force-pushed the feat/mf-deep-imports branch from 05685f6 to 120252d Compare October 9, 2023 14:33
@RafikiTiki RafikiTiki force-pushed the feat/mf-deep-imports branch from 120252d to 7c6808e Compare October 9, 2023 14:35
@RafikiTiki RafikiTiki merged commit 7deea25 into main Oct 9, 2023
@RafikiTiki RafikiTiki deleted the feat/mf-deep-imports branch October 9, 2023 15:42
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

Successfully merging this pull request may close these issues.

5 participants