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

Hack: patch Metro to not watch files when bundling #873

Merged
merged 4 commits into from
Apr 16, 2019

Conversation

hypest
Copy link
Contributor

@hypest hypest commented Apr 15, 2019

We seem to have reached some limits with file watchers in JitPack when building the Android JS bundle there (to use in WPAndroid) and that started after #844 got merged.

See a failed JitPack log here: https://jitpack.io/com/github/wordpress-mobile/gutenberg-mobile/26013ee0d5/build.log

This PR introduces a nasty hack to fix this. I couldn't find a practical way to stop Metro from watching for files while bundling (not even sure why would it need do the watching at that phase, maybe a bug with the React Native bundle command?) and couldn't alter JitPack's default limit for inotify watchers. The hack here is to patch Metro by copying DependencyGraph.js.patched over the original, just to change the watch: true flag to false.

I will re-use the WPAndroid PR (wordpress-mobile/WordPress-Android#9614) to test this out.

To test:

The WPAndroid PR should just be green as the build should succeed.

@hypest hypest added this to the v1.3 milestone Apr 15, 2019
Copy link
Contributor

@marecar3 marecar3 left a comment

Choose a reason for hiding this comment

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

@hypest LGTM!

@hypest hypest merged commit 3a48395 into develop Apr 16, 2019
@hypest hypest deleted the fix/dont-watch-files-on-bundling branch April 16, 2019 07:41
@hypest
Copy link
Contributor Author

hypest commented Apr 19, 2019

Looping back, I brought up here the fact that we're affected by Metro's behavior to always watch the files, even when RN is performing the bundling procedure.

ceyhun added a commit to WordPress/gutenberg that referenced this pull request Jun 25, 2020
This was introduced in wordpress-mobile/gutenberg-mobile#873
as hack to patch metro to not watch files when bundling. Current 0.58.0 version of
metro that we are using now automatically checks if it's running on CI via 'ci-info'
npm package and doesn't enable watch.

See: https://www.npmjs.com/package/ci-info
See: https://github.com/facebook/metro/blob/v0.58.0/packages/metro/src/node-haste/DependencyGraph.js#L96
hypest pushed a commit to WordPress/gutenberg that referenced this pull request Jun 26, 2020
…23452)

* Print system information

* Add debugger to iOS workflow

* Remove non-working npm cache clean command

* Remove android runner

* Install watchman

* Bundle iOS 25 times

* Bundle iOS 25 times w/o watchman

* Bundle iOS 25 times with watchman

* Try one last time without watchman

* Remove maxWorkers parameter from metro config

* Remove metro DependencyGraph.js patch

This was introduced in wordpress-mobile/gutenberg-mobile#873
as hack to patch metro to not watch files when bundling. Current 0.58.0 version of
metro that we are using now automatically checks if it's running on CI via 'ci-info'
npm package and doesn't enable watch.

See: https://www.npmjs.com/package/ci-info
See: https://github.com/facebook/metro/blob/v0.58.0/packages/metro/src/node-haste/DependencyGraph.js#L96

* Revert "Remove android runner"

This reverts commit 51ba723.

* Install watchman on both iOS and Android runners

* Remove script bundling iOS multiple times

* Run React Native E2E tests on pull request instead of push
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants