feat(CLI): deep merge Metro serializer

[tjzel]

Summary:

Running metro via community CLI's start command triggers overriding of the Metro config, namely resolver and serializer. While developing react-native-worklets library I found it useful to add custom logic to resolver and serializer to enable more advanced features. The config will remain overridden by default and can be explicitly disabled by the user.

Adding this to the CLI plugin might be too high up in the logic, if you think it would be better to apply this change somewhere else, please guide me 🙏

Changelog:

[GENERAL] [ADDED] - Add option to disable Metro config override on demand

Test Plan:

You can run yarn start and yarn react-native bundle with and without --disable-config-override in the RNTester app and see that it works well.

[huntie]

I'm curious which resolver and serializer properties you are overriding that aren't correctly merged currently?

Conceptually, this seems right if there's a clear need — we'd been moving towards metro.config.js as the only source of truth. Will tag @robhogan on the imported code review.

[tjzel]

I don't utilize resolver just yet, but I will probably have to later - I'm creating files ad-hoc during Babel transpilation of some files and those files sometimes aren't resolved by metro, a reload helps there.

Speaking of serializer, I do use getModulesRunBeforeMainModule PR - check metro.config.js. The reason for that is when I spawn another Hermes runtime, I want to initialize it with my custom module and also skip the initialization from React Native. To have access to all the libraries and polyfills, I reuse the main bundle. On the React Native runtime my module simply returns early and induces no changes.

[huntie]

@tjzel Interesting. I guess we could merge more correctly, e.g. this:

Looks like resolveRequest will be deep-merged already — it's only platforms that gets hard-overriden by the CLI input args (makes sense).

  serializer: {
      // We can include multiple copies of InitializeCore here because metro will
      // only add ones that are already part of the bundle
      getModulesRunBeforeMainModule: () => [
        require.resolve(
          path.join(ctx.reactNativePath, 'Libraries/Core/InitializeCore'),
          {paths: [ctx.root]},
        ),
        ...outOfTreePlatforms.map(platform =>
          require.resolve(
            `${ctx.platforms[platform].npmPackageName}/Libraries/Core/InitializeCore`,
            {paths: [ctx.root]},
          ),
        ),
+       ...(config.serializer.getModulesRunBeforeMainModule() ?? []),
      ],
    },

[tjzel]

A deep merge would be good too. I tested it for my use case and it worked, I only had to change the order so React Native's initializer wouldn't be first. Although it makes more sense in this case to be able to choose if you want your initializer to be prepended or appended.

--- a/dist/utils/loadMetroConfig.js
+++ b/dist/utils/loadMetroConfig.js
@@ -33,6 +33,7 @@ function getOverrideConfig(ctx, config) {
     resolver,
     serializer: {
       getModulesRunBeforeMainModule: () => [
+        ...(config.serializer.getModulesRunBeforeMainModule() ?? []),
         require.resolve(
           _path.default.join(
             ctx.reactNativePath,

[robhogan]

I'm creating files ad-hoc during Babel transpilation of some files

I'd recommend against this in general. It's not a good idea for transforms to have side-effects (or untracked inputs) because there are no guarantees that your new files will stay in sync.

What are you working on? There might be a better way, or it might fit the virtual module system in development atm.

[tjzel]

@robhogan I opened a separate issue for the file creation problem facebook/metro#1488

To put it simply, I want to generate web-worker like environment implicitly for the user - that meaning, creating a separate file for any function that could be used on a separate Hermes Runtime, beside the one spawned by React Native. I figured that to support hot reloads I need to hook into Metro bundling process - build step doesn't seem to be an option here.

[tjzel]

Hey @huntie, so what's the direction we want to follow in this PR?

[huntie]

@tjzel Let's go for the serializer deep merge, now you've validated that. Ideally, we want as little extra config as possible — so this implicit config merging fix makes sense 🙏🏻

[tjzel]

@huntie I added deep merging, but getModulesRunBeforeMainModule expects a string argument of the entry file path, which isn't used and isn't actually available from the pipeline. I've thrown a dummy string to pass the tests, is it okay?