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

rnx-bundle runs the metro Server without "watch:false" #2332

Open
acoates-ms opened this issue Apr 7, 2023 · 5 comments
Open

rnx-bundle runs the metro Server without "watch:false" #2332

acoates-ms opened this issue Apr 7, 2023 · 5 comments
Labels
bug Something isn't working feature: metro This is related to Metro

Comments

@acoates-ms
Copy link
Contributor

The command ends up either in @react-native-community/cli-plugin-metro buildBundleWithConfig or in @rnx-kit/metro-service calling new Server(config).

When doing a single bundle command it should use new Server(config, {watch:false}) to prevent the extra work of spinning up the jest-haste-map file watcher.

@tido64 tido64 added bug Something isn't working feature: metro This is related to Metro and removed needs triage 🔍 labels Apr 10, 2023
@tido64
Copy link
Member

tido64 commented Apr 10, 2023

Is this all we need to do?

diff --git a/packages/metro-service/src/bundle.ts b/packages/metro-service/src/bundle.ts
index fc994559..82b7f947 100644
--- a/packages/metro-service/src/bundle.ts
+++ b/packages/metro-service/src/bundle.ts
@@ -87,7 +87,7 @@ export async function bundle(
     platform: args.platform,
     unstable_transformProfile: args.unstableTransformProfile,
   };
-  const server = new Server(config);
+  const server = new Server(config, { watch: false });

   try {
     const bundle = await output.build(server, requestOpts);

@tido64
Copy link
Member

tido64 commented Apr 11, 2023

On closer inspection, @react-native-community/cli-plugin-metro also starts the watcher:

https://github.com/react-native-community/cli/blob/6fd0e2e847d5db93b82e259dbb75931e6a3aaa7e/packages/cli-plugin-metro/src/commands/bundle/buildBundle.ts#L92

So I'm not sure this is always correct. Maybe this makes sense when you're on CI? Need to check whether the watcher persists.

@acoates-ms
Copy link
Contributor Author

The only thing I can think of is that in the case of a watchman watcher, it probably launches watchman, which would persist. The node watcher, and the node part of the watchman watcher I dont think persists.

@acoates-ms
Copy link
Contributor Author

I think this probably should be fixed in the community cli too.

@tido64
Copy link
Member

tido64 commented Apr 11, 2023

The only thing I can think of is that in the case of a watchman watcher, it probably launches watchman, which would persist. The node watcher, and the node part of the watchman watcher I dont think persists.

I think locally, this makes sense. People can reuse the instance if they need to run bundle again, or switch between dev server and using bundles (I know I do this a lot). Turning it off seems wrong. I've also been told that either CLI or Metro automatically disable watching when it detects it is on CI, so we shouldn't have to do anything here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature: metro This is related to Metro
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants