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

Add watch flag to metro config #491

Closed
wants to merge 2 commits into from
Closed

Conversation

cdsanchez
Copy link

@cdsanchez cdsanchez commented Dec 5, 2019

Summary

This is an attempt enable users of Metro to configure the watch flag that is passed to JestHasteMap. This addresses #355

Without this change, we see failures on some of our internal build machines where we cannot easily configure fs.inotify.max_user_watches.

Test plan

I have a corresponding PR to the react-native-cli: react-native-community/cli#879. I used that change (via yarn link) to test this change in metro.

Invoked metro via react-native start

  • Observed that the watch flag was true through console.log

Invoked metro via react-native bundle

  • Observed that watch was set to false.

If there are any other test cases you would like me to run, please let me know!

@facebook-github-bot
Copy link
Contributor

Hi cdsanchez! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@codecov-io
Copy link

codecov-io commented Dec 5, 2019

Codecov Report

Merging #491 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #491   +/-   ##
=======================================
  Coverage   84.46%   84.46%           
=======================================
  Files         173      173           
  Lines        5820     5820           
  Branches      961      961           
=======================================
  Hits         4916     4916           
  Misses        801      801           
  Partials      103      103
Impacted Files Coverage Δ
packages/metro-config/src/defaults/index.js 85.71% <ø> (ø) ⬆️
packages/metro/src/node-haste/DependencyGraph.js 83.33% <ø> (ø) ⬆️
packages/metro-config/src/oldConfig.js 73.33% <ø> (ø) ⬆️
packages/metro-config/src/convertConfig.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ad7bb4...1ec20f7. Read the comment docs.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 18, 2019
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@cpojer
Copy link
Contributor

cpojer commented Dec 27, 2019

Thank you for your PR. There is a similar PR with #497. Let's use that one to discuss further.

@cpojer cpojer closed this Dec 27, 2019
facebook-github-bot pushed a commit that referenced this pull request Jan 10, 2020
Summary:
**Summary**

In this PR I am disabling file watching for bundle generation done through `runBuild`. A `watch` option is also added to `Server` in order to enable that (and also because `Server` is used by the React Native CLI instead of `runBuild`).

The goal of this is to solve `ENOSPC` errors related to excessive file watcher use, particularly in CI environments. See: #355

Context:
I previously attempted to fix this by resolving a regression in the existing `watch` config option: #491
Which was closed in favor of a similar PR:  #497
Which was then re-created in: #503 (and landed)

The above PR was modified to automatically set the `watch` value depending on whether it was built in a CI environment. There was a desire to remove the `watch` config option altogether.

At the end, we concluded that there is still a need for a caller to turn off file watching explicitly, and not only through CI detection.

Therefore, I'm threading it through the code as an option that is passed through programmatically and *not* through the config.

Once/if this lands, I will update the React Native CLI to use this option.

**Test plan**

I used the rn-cli as a test harness of sorts.

Invoked metro via `react-native start`
- Observed that the `watch` flag was true through `console.log`

Invoked metro via `react-native bundle`
- Observed that `watch` was set to false.

Relatively new to this project, so please let me know if I should be taking a look at anything else.
Pull Request resolved: #507

Differential Revision: D19341662

Pulled By: cpojer

fbshipit-source-id: d0430364d57fefe027f6b1f58f964d9765cda752
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants