-
Notifications
You must be signed in to change notification settings - Fork 636
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
Watch option is ignored when creating JestHasteMap #355
Comments
This seems to have been introduced by FB internal changes here: 0d6c135 . We were hoping to use the I'm not very clear on the metro/haste-map internals, but it seems odd that a static build (when doing Is there any context as to why this config option is ignored as of 6 months ago? |
Sorry for the possible noise on a ticket that might be unrelated but, wanted to add that similar to @FLGMwt , our project needs to have the Edit: for the project I'm currently working on, I've put a ugly nasty solution in place: to replace the |
I know nothing of react-native but have been tasked with figuring out the same max_user_watches issue when building a react-native project in CI. I can't think of a reason file watchers for js builds should be needed when just bundling for CI. As I started digging through the rabbit hole of react-native, the cli, and these tools, I arrived at the watch metro config option. Despite 0d6c135 it would seem the docs still mention watch as an option. Are there any alternatives to short of the above suggestion about replacing DependencyGraph? I'm also interested in the context of why this option was removed @rafeca @mjesun |
Thank you for the solution @hypest! This has been a huge issue for us for over a week breaking our CI builds and nothing worked except patching this file directly. I had to update the patched source file since it has changed since your reference PR, but this approach did work - thank you. I made this changed in the patched file: // watch: opts.watch, (line 106 in DependencyGraph.js)
watch: false, Forcing I don't know why disabling watch was not respected, it would be helpful for this to be fixed. |
@rafeca @mjesun I think it is very unreasonable to enforce watching project files even in publish mode, since filesystem watches consume quite amount of system resource that could have been devoted for faster builds. It is also breaking continuous integration pipelines when it needs to unnecessarily watch ridiculous number of files, especially in |
We tried updating the module.exports = {
watch: false,
}; to disable watch mode, according to the documentation here https://facebook.github.io/metro/docs/en/configuration#watch but neither worked. If either of these configurations was incorrect, I'm happy to try again with the correct configuration. For reference we are using Metro via Expo for a React Native project. We also tried other options to disable watch mode using the Expo CLI, but these did not work as well. |
before update release, I have to use the patch in this way:
replace the |
This is causing issues on internal custom CI servers as well, we need a way to switch off watch via configuration. |
@kinka great fix, thanks for sharing. Using
|
I've now packaged this hack as a node module: https://www.npmjs.com/package/react-native-nowatch. Hope this helps someone out there - I've had a really frustrating time getting react-native builds working on CI, and the suggestions in this thread really helped. Thanks to all above. |
#503) Summary: **Summary** *Issue : #355 *Follow up on my old PR : #497 The RN app build process is failing in CI servers due to the always-on file watch of metro. Mostly the CI servers will have a limit on no. of file watch at a time. This is a problem for RN apps with large no. of dependencies, and thus with a large no. of files in `node_modules`. Which cause the build to fail with `ENOSPC` error. This PR will detect the CI servers with [`ci-info`](https://github.com/watson/ci-info/) module and turn off the [file-watching of JestHasteMap in metro](https://github.com/facebook/metro/blob/f6314e43071ae498b41d9e5579ad4a13941a4baa/packages/metro/src/node-haste/DependencyGraph.js#L95). ~~Also in the [Metro documentation we have a `watch` field](https://facebook.github.io/metro/docs/en/configuration#watch), but it is not working since [this commit ](0d6c135#diff-75dc57ca2d6ab410090adb917784434f). So this PR will also make it working, so that people who wish to turn off the watch can do that with their metro config.~~ And this change is a no-op to current metro users, as it will maintain the default watch behavior. Also we can manually turn off the file watch by setting the `CI=true` environment variable. **Motivation** *Why this fix is required now ?* This file watching functionality will help in accelerating the local development, but that is not expected when we are building the project to release in our CI pipelines, where the build is happening in some docker containers or some ec2 servers, where sometimes we do not have access to set the `fs.inotify.max_user_watches`. So people try the workarounds like, npm script for deleting the folders in the node_modules which are not required in the bundling process. **Test plan** Completing the [metro pull request workflow](https://github.com/facebook/metro/blob/master/CONTRIBUTING.md#workflow-and-pull-requests). And I tested the functionality by creating a [sample RN app](https://github.com/alanjoxa/AwesomeProject) by applying this [patch](https://github.com/alanjoxa/AwesomeProject/tree/master/patches) to its metro dependencies in node-modules, as part of its [postinstall script](https://github.com/alanjoxa/AwesomeProject/blob/e52b92fd04d61e9ed8530d5fdf7f59657b1a8a2e/package.json#L11). It is working as expected. **Test simulation instructions** ``` git clone https://github.com/alanjoxa/AwesomeProject.git cd AwesomeProject npm install npm start ``` This will start a metro server and serve the bundles by watching all files in the project root. We can turn off the file watch by restarting it by `CI=true npm start`. A sample bundle can be triggered by making a GET request at `http://localhost:8081/index.bundle?platform=ios`. When in - `npm start` - Metro server will watch for all files in the project folder, including `node_modules` and apply any file changes to the bundle on the subsequent build request. - `CI=true npm start` - Will not watch for files, that means we need to restart the server for seeing the file changes. Pull Request resolved: #503 Differential Revision: D19286561 Pulled By: cpojer fbshipit-source-id: 68f99f746440ab2e199189bba92489a79e800f5d
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
So, after all pull requests above... There seems to be an easier solution now. Just add the |
Yes - this was essentially resolved by #507 but this issue was just never closed - |
Do you want to request a feature or report a bug?
bug
What is the current behavior?
watch is hardcoded to true when creating JestHasteMap: https://github.com/facebook/metro/blob/master/packages/metro/src/node-haste/DependencyGraph.js#L96
If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can
yarn install
andyarn test
.https://github.com/hfter789/metro
jest packages/metro/src/integration_tests/__tests__/buildGraph-test.js
What is the expected behavior?
JestHasteMap should receive the
watch
value coming from metro config.The text was updated successfully, but these errors were encountered: