Skip to content

Conversation

jbroma
Copy link
Member

@jbroma jbroma commented May 28, 2025

Summary

This PR adds ability to provide custom implementation for getModulesRunBeforeMainModule through Metro config, allowing other modules to run before InitializeCore. Current implementation revolves around checking if the InitializeCore path was already included in the array and if not, it get's prepended to the modules list.

Motivation

loadMetroConfig always overrides the original getModulesRunBeforeMainModule and there is no way to change it currently

Alternative

we might also completely opt out of modifying this if the user overrides this in his Metro config

Test plan

n/a

Copy link

vercel bot commented May 28, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
rnef ⬜️ Ignored (Inspect) May 28, 2025 9:53am

@jbroma
Copy link
Member Author

jbroma commented May 28, 2025

@thymikee @szymonrybczak since you are the RNC CLI bros, what do you think of this solution? Should we maintain this behaviour of prepending modules or just straight up opt out of modifying the modules here if the user provides a custom implementation?

@jbroma
Copy link
Member Author

jbroma commented May 28, 2025

I believe there is no other way other than the current implementation (with checks & prepending) because getDefaultConfig from metro-config here: https://github.com/facebook/metro/blob/6f0093c4cdc597dbbde8b22b159b9cb0b7912865/packages/metro-config/src/defaults/index.js#L66

@react-native/metro-config also defines it: https://github.com/facebook/react-native/blob/a6f24a5eaa2e304a164d4feae473b464fe5e60f6/packages/metro-config/src/index.flow.js#L58-L60

if we would opt out of overriding the getModulesRunBeforeMainModule when it's present, we would break most setups

Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Makes sense, didn't realize you couldn't override this in user land before

@jbroma
Copy link
Member Author

jbroma commented Jun 4, 2025

opened the same PR in RN Core: facebook/react-native#51820

let's wait for that one to be merged and then we can align with that here 👍

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.

2 participants