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

fix(metro-service): avoid spinning up the file watcher #2334

Closed
wants to merge 1 commit into from

Conversation

tido64
Copy link
Member

@tido64 tido64 commented Apr 10, 2023

Description

Avoid spinning up the file watcher when bundling.

Resolves #2332.

Test plan

n/a

@tido64 tido64 requested a review from acoates-ms April 10, 2023 16:44
@github-actions github-actions bot added the feature: metro This is related to Metro label Apr 10, 2023
@tido64 tido64 enabled auto-merge (squash) April 10, 2023 16:44
@@ -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 });
Copy link
Contributor

Choose a reason for hiding this comment

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

After this change will we be able to still spin up the file watcher if we want? Rather than hardcoding this as false, shall it make sense to take this option as an input?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why would you want to watch the files for a bundle command?
If you are starting a metro server then it would make sense. But I believe this code is only hit in the single bundle case, in which case spinning up a watcher is just overhead.

@acoates-ms
Copy link
Contributor

In OMR at least we were hitting this case at the top of the function, so this change would not be sufficient.

  const buildBundleWithConfig = getBundlerFromCliPlugin();
  if (buildBundleWithConfig) {
    return buildBundleWithConfig(args, config, output);
  }

@tido64 tido64 marked this pull request as draft April 14, 2023 12:01
auto-merge was automatically disabled April 14, 2023 12:01

Pull request was converted to draft

@tido64
Copy link
Member Author

tido64 commented May 23, 2023

Closing as this fix doesn't apply to newer versions of the CLI, and I don't have time to do proper testing.

@tido64 tido64 closed this May 23, 2023
@tido64 tido64 deleted the tido/bundle-watch branch May 23, 2023 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: metro This is related to Metro
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rnx-bundle runs the metro Server without "watch:false"
3 participants