Summary: RN configuration allows to specify a post-minify hook. This wasn’t called in the new Buck integration so far. Added here.
Reviewed By: cpojer
Differential Revision: D5087325
fbshipit-source-id: 74b58bd3a203716d8f01b5d7ba552b6ad1b575ce
Summary: Internally we use `multipart/form-data` to upload transformed files to the global cache, using the keys as file names. Unfortunately the server would read that and consider only the basename as the key, not the full path. So we wouldn't store the data under the right key. This is definitely a bug in the way upload is implemented: we should transmit the keys separately. But, this changeset offers a quick mitigation by avoiding slashes in the key, so that the whole key is a valid base file name.
Reviewed By: davidaurelio
Differential Revision: D5087780
fbshipit-source-id: 9e4a698c1f57c4c3b91b56b43eef82c1c7dd862b
Summary: Better to have *some* Flow coverage (I'd like 100% of packager covered from when we split it apart) than none.
Reviewed By: davidaurelio
Differential Revision: D5077757
fbshipit-source-id: b23169b3edf2bd3eb0e8a399d099151aa705b198
Summary:
It was hard to type the resolution main algo, I had to put type annotations explicitely everywhere, otherwise Flow would get in some kind of loop and do weird errors. I think it's because the algo is recursive and Flow tries to infer types too deeply because of the generics.
Anyway, apart from that it's good to get this extra type security in the few other places. We require Node v4 minimum, that according to the internets supports the `class` syntax without transform, and I verified that inheriting from `Map` actually works as expected.
Reviewed By: davidaurelio
Differential Revision: D5078023
fbshipit-source-id: 05dfc4acf5b07cdda8a7b36ec9cba216d1810643
Summary: Or, a case in point that Flow doesn't saves us from all the ills. I think it didn't complain because `MapWithDefaults` doesn't have proper typing. I'll deal with that separately.
Reviewed By: davidaurelio
Differential Revision: D5077707
fbshipit-source-id: c43623c5046d2dea9964685a44ad4877d060232e
Summary: I'd like to start typing the front-end so that it's easier to track adding new options, etc.
Reviewed By: davidaurelio
Differential Revision: D5069868
fbshipit-source-id: 9a18dca52efd486ca18f17d0ec434a5ec1c1649c
Summary: This makes it easier to verify correctness when adding new config/args.
Reviewed By: davidaurelio
Differential Revision: D5069537
fbshipit-source-id: 4d8058851900b23163d0f2744e91dd14dfcdd461
Summary:
The existing resolution logic of assets:
* goes over all the files of the asset's directory for every resolution request;
* duplicates the parsing logic of `AssetPaths` by building up a custom regex for each resolution request.
This changeset proposes to tweak this by building an index for each particular directory in which we're looking for assets, so that we don't have to crawl a single directory twice, and so that it reuses the logic of `AssetPaths.tryParse()` for determining variants.
Reviewed By: davidaurelio
Differential Revision: D5062435
fbshipit-source-id: 708fc5612f57b14565499fad741701269438c806
Summary:
We appended a `sourceMappingURL` in the same place where we write out the files. This will break output that is not a plain text file, like Random Access Bundles.
This moves the corresponding logic into the function that builds the bundle.
Reviewed By: cpojer
Differential Revision: D5061039
fbshipit-source-id: 17fadb5a687c8d4b1f29439e8bf946bae58eb2d9
Summary:
Not only is this function is building a custom Regex for every single file, but it's also duplicating some of the work of the inner function, that is already splitting up the platform/extension. This changeset refactors both function to have a more strict and legible logic to extract asset information. We extract the base name first, then we extract the resolution from it, instead of rematching platform and extension.
I stumbled on this while looking into refactoring the asset resolution logic of `ResolutionRequest#_loadAsAssetFile()`. The next step would be to reuse the exact same function for resolving assets instead of using a custom regex there as well.
Reviewed By: davidaurelio
Differential Revision: D5060589
fbshipit-source-id: b48d9a5d8e963be76cad5384c6bfb3e214a73587
Summary: Add Flow types, revealing a few problems, such as `isHaste` having the wrong return value in the "pseudo-mocks". But since the buck worker is in fact working, I guess these functions were never called... The point of typing this file is that I'm going to start aggressively pruning dead code in `node-haste` and hopefully, eventually, get rid of `Moduleish` and `Packageish`.
Reviewed By: davidaurelio
Differential Revision: D5052379
fbshipit-source-id: dab3f18f05fcf43fbbc48b589170b1cf367d6a48
Summary: This does not appear to be used anywhere anymore.
Reviewed By: cpojer
Differential Revision: D5052224
fbshipit-source-id: 142fdcdf48f4ad16a04bb747b41d623f214f7a68
Summary: Default arguments are dangerous, and it shows here again. `Server` was calling that function without passing down the platforms, meaning custom platform would not be accounted for, possibly causing all kinds of bugs for OSS use cases. Additional, the typing of `platform` across the stack was wrong: it can be `null`, in which case we resolve the files without extension. The reason Flow didn't detect that issue before is because we use `Object.assign` to re-export the function from `DependencyGraph`, but Flow is not smart enough to propagate the types in that particular case. I'll remove all the other re-export, as it may uncover further type errors.
Reviewed By: davidaurelio
Differential Revision: D5052070
fbshipit-source-id: 7b427ec036ca74b5dcd7c88f7dfa0df541b8eb6b
Summary: in new-style builds, the prelude is not wrapped into an IIFE, so `global` is not available. Since the bundle as a whole is not running in strict mode, we can access the global object with `this`.
Reviewed By: jeanlauliac
Differential Revision: D5051731
fbshipit-source-id: 4003b5e57ba8626e38e68e92d5778c2c59ca69a5
Summary:
A quick fix to ensure RNTest works. This PR changes the process root for where packager is run, resulting in the right `cwd` directory path when RN launches RNTester.
Closes https://github.com/facebook/react-native/pull/13947
Differential Revision: D5052093
Pulled By: davidaurelio
fbshipit-source-id: 506a8cd63f1709c948bcc4586467020d380ba85e
Summary: I stumbled on this while refactoring that function, and i realised that, I believe it doesn't make sense to take into account the platform extension of the "potiential" file path. The reason is, if you try to require "foo.ios.png", the returned asset name would be "foo", and thus we'd try to find "foo.${ext}.png" where `ext` could actually be `android` or anything else! So it's confusing. There's no reason we should allow callsites to specify platform anyway I think. With this changeset we're not losing any functionality, but it might require people to fix some incorrect callsites.
Reviewed By: cpojer
Differential Revision: D5051791
fbshipit-source-id: 2a1ec7a8bfa6791b6016213305a72bc0b81f23b9
Summary: I found myself a few times wanting that transform, that makes it slightly simpler to have bound method. So I propose we add it. Not a big deal though. Note it also allows static properties with the same syntax, that is handy.
Reviewed By: davidaurelio
Differential Revision: D5051579
fbshipit-source-id: 7ebf7c709bf52a30a525550c1eda1a6a2f7b8e1e
Summary:
Pass `localPath` to transformers to allow usage of plugins like `transform-react-jsx-source` that don’t conflict with x-machine caches, e.g. Buck or our own global cache.
These caches don’t work properly if paths local to a specific machine are part of cached outputs.
Reviewed By: jeanlauliac
Differential Revision: D5044147
fbshipit-source-id: 1177afb48d6dd9351738fcd4da8f9f6357e25e81
Summary:
Changes the global cache to use *local paths* rather than base names of files for the cache key. This is to enable correct usage of babel transforms like `transform-react-jsx-source` that use file paths in their output.
It remains a responsibility of the transform implementer to pass relative paths to babel if the global cache is being used.
Reviewed By: jeanlauliac
Differential Revision: D5044028
fbshipit-source-id: 2ef1e1545e510a18ab49a307053d91b4f40269b2
Summary: `transformModulePath` used to be an optional string, because `ConfigT` allowed for an optional `getTransformModulePath` method. Effectively, we required it to be present. This builds on top of the cleanups around `ConfigT` and gets rid of the flow error suppressions
Reviewed By: jeanlauliac
Differential Revision: D5037466
fbshipit-source-id: bc5c9cbc566e7aa74e7f6397e69fa87cdac7bc00
Summary: That's a bit of an experimental changeset, I wanted to experiment how we can track with more precision the resolution and this is what I come with. That allows us to know what has been tested if we canot find a match. It also uses a simpler control flow with early `return`s all across. Finally, this reflect the strategy I want to apply for the rest of the resolution. (Right now the error is still not great, as it may be deeply nested, as in the case of packages resolution.)
Reviewed By: davidaurelio
Differential Revision: D5044040
fbshipit-source-id: 2e256174506f80d90ee83175057666d530785788
Summary: Internally when adding `format` we have a lint rule that automatically reformat using `prettier` and the project rule. I'm concerned how we can ensure this stays consistent when merging PRs though. It's not a big issue because the volume of PRs is low, but we'll have to figure it out later on.
Reviewed By: davidaurelio
Differential Revision: D5035830
fbshipit-source-id: 6f2bc9eb8212938ff785a34d2684efd1a9813e1a
Summary: That module is not used anymore, remove it and its dependency `joi`.
Reviewed By: cpojer
Differential Revision: D5028909
fbshipit-source-id: 90b9b156fbfe642cce93a530faf8ce91c5b848f5
Summary: Adds flow type defs for uglify, and fixes the two issues found
Reviewed By: jeanlauliac
Differential Revision: D5029190
fbshipit-source-id: eb5947b051844938da241e002b727edc1384e3a6
Summary: This removes the single-use "garbage collection" class and make a single `TransformCache` instead. The reason for doing this is that I'd like to experiment with using a local dir based on the root path instead of the `tmpdir()` folder, because on some platform using the temp dir is subject to permissions problem, sometimes it is empty, and on all platform it is vulnerable to concurrency issues.
Reviewed By: davidaurelio
Differential Revision: D5027591
fbshipit-source-id: e1176e0e88111116256f7b2b173a0b36837a887d
Summary: I'm continuing my changes to avoid `lstat`-ing the folders when we don't really need to. That changeset in particular proposes to remove the check in `_loadAsDir`. The rationale is that right after that we try for the presence of the `package.json`. If that file exists, the folder necessarily exist so we can switch these two `if` blocks for sure without changing the logic. Finally, at the end we look for the "index" file. By removing the folder check, packager would now report "file `foo/index` does not exist" rather than "directory `foo` does not exist" if the folder does not exist. I think it's not much worse, especially as both of these are unhelpful for what I believe to be a large number of cases already anyway. Indeed, the whole algo is based on a series of try/catch, and only the very last try will see its error surfaced. But, most often, it'd be useful for earlier tries to be surfaced to the user. I do want to improve that; meanwhile, however, I sense it's not a big deal to remove that folder check for all these reasons. If you don't agree, I do have another proposition: we could catch errors generated by the last `_loadAsFile` call, and rethrow them as directory errors instead (if `dirExists` return `true`). That way, the call to `dirExists` would only happen in case of errors (but that could still happen quite a lot as a result).
Reviewed By: davidaurelio
Differential Revision: D5028341
fbshipit-source-id: 2d4c99c0f352b71599482aa529728559466b76fd
Summary: I think we don't really need to check the directory beforehand, the function to find asset will just return an empty array. As for the error message, I tried adding a require of an asset file somewhere, but due to the way the ResolutionRequest algo work, it generates a final error message that unrealted ("Directory ... does not exist", instead of the "asset does not exist"). I plan to revamp the way errors are handled such as the error message clearly identifies what file paths have been inspected.
Reviewed By: davidaurelio
Differential Revision: D5028118
fbshipit-source-id: 496472001c0a3d4192bfef4a0c8a0dc8a9a0fa82
Summary: This is not used by live code anymore.
Reviewed By: cpojer
Differential Revision: D5029114
fbshipit-source-id: 9ab9f6075407623debfe23bc121cc48ae8903917
Summary: I've been confused for a long time by this, and I think it's better late than never. I propose we rename that file to make it more explicit where that class lives, and so that it's consistent with the test file, name `DependencyGraph-test.js`
Reviewed By: davidaurelio
Differential Revision: D5020556
fbshipit-source-id: d54a501c3995f3fea16a5bfc6ca72993f73c4873
Summary: Moves custom transform options into their own property when returning custom properties. This helps both transitioning to a `select` function as well as implementing RAM bundle support for the new buck integration
Reviewed By: jeanlauliac
Differential Revision: D5028075
fbshipit-source-id: 25fe3072023cc063deef537a12012d4bb3173579
Summary: Afaik. the list of asset types is hardcoded for the Buck worker so far, and I'm not sure how that would be customizable. For now, let's include all the default asset extensions.
Reviewed By: davidaurelio
Differential Revision: D5002607
fbshipit-source-id: 41069e817d2b73156bca684bc2f73077132928a7
Summary:
There was an error with packager symbolization on Windows because it looks like `allowHalfOpen: true` doesn't work properly when using named sockets. This uses a random port on localhost for the connection instead on Windows as a workaround.
**Test plan**
Tested that symbolization works on both mac and windows by triggering a warning in UIExplorer.
Closes https://github.com/facebook/react-native/pull/13828
Differential Revision: D5019537
Pulled By: davidaurelio
fbshipit-source-id: 62c5b5f270e553a7d413bb4d1bab2406380f1d4f
Summary:
- [x] Explain the **motivation** for making this change.
- [x] Provide a **test plan** demonstrating that the code is solid.
- [x] Match the **code formatting** of the rest of the codebase.
- [x] Target the `master` branch, NOT a "stable" branch.
I have a need to bundle a pre-optimized external lib with my RN application. Until RN 0.42 I had been using a .babelignore to prevent the packager from trying to optimize this file and choke.
It seems in 0.42 and higher I'm no longer allowed to ignore the file.
This issue has also been reported as #12071
Details on the reasoning for this patch can be found in the issue I originally filed: https://github.com/facebook/react-native/issues/13168
What existing problem does the pull request solve?
This PR restores the functionality with babel ignoring files that existed in 0.41 before this patch:
0849f84df2 (diff-4676ea0b3c55c65c3929aa993144f07f)
Here's a screenshot of this patch properly ignoring the file I referenced in https://github.com/facebook/react-native/issues/13168 to be ignored.
The patch relies on the `ignored` value of the call to `babel.transform` and if true returns the src in a object per instruction from loganfsmyth from BabelJS core team.
To test, add a file to the `ignore` array of a `.babelrc` file in a React Native project with this fork.
I was unable to locate a test file for the transformer.js
Fixes#12071, #13168
Closes https://github.com/facebook/react-native/pull/13681
Differential Revision: D5017565
Pulled By: davidaurelio
fbshipit-source-id: 421f57b5ce192eedd46fae4285d8a741cb5f8e71
Summary: Well that's silly but I noticed with faster terminal progress updates, packager *feels* a bit faster :D
Reviewed By: davidaurelio
Differential Revision: D5002528
fbshipit-source-id: 8a3d5e929dd7fb09ebafda3422886c631016a40f
Summary: Fixes building release builds with the new buck integration.
Reviewed By: jeanlauliac
Differential Revision: D5002441
fbshipit-source-id: e7716324c7c8dd0bfcaf5b39cf716ac589e948e8
Summary: Enforces a `'default'` property to be present on the transform variant mapping. This will allow us to simplify transforms of assets and json to only provide one variant for the new Buck build system.
Reviewed By: jeanlauliac
Differential Revision: D5002052
fbshipit-source-id: 2a7240c1b2450f62de686c46ab2c2e5a75dea399
Summary:
I suggest we grab our own version of worker-farm, since there are a few changes we'd like to do. There are two reasons for forking:
* the original project does not seem maintained anymore, with a PR remaining unanswered (https://github.com/rvagg/node-worker-farm/pull/42);
* we don't need to keep the level of genericity of the original project: for example, we don't need the option `maxConcurrentCallsPerWorker`, that we always keep to one.
Forking gives us opportunity to simplify the code for our use case. Later on we could reuse it for other projects such as `jest`.
A few things we'd like to do:
* remove special node options from the forks, such as `--inspect`, or even, allow adding special options (if you want to debug a worker specifically for example);
* allow us to pipe `stdout` and `stderr` instead of having transform spit stuff out to the parent process output;
* remove code managing `maxConcurrentCallsPerWorker` and clean up the code in general;
* add `flow` typing.
Reviewed By: davidaurelio
Differential Revision: D4993300
fbshipit-source-id: 10f0c2a18b010c2a8b2e2afebcb3aab3504d7923
Summary: `no-alert` doesn’t play nice with flow type spreads. We don’t need it for node, anyway.
Reviewed By: jeanlauliac
Differential Revision: D4993096
fbshipit-source-id: 95785843d3263520c063a43864c8053cbaa5083d
Summary:
`declareOpts` prevents strong Flow typing, and promotes default values, that may be different from a function to another (my goal is to remove defaults from the whole codebase except perhaps the public API). This changeset replaces it by Flow types and fixes callsites to be explicit on values.
This is the last callsite of `declareOpts` so I'll remove it, in a separate diff.
Reviewed By: cpojer
Differential Revision: D4970650
fbshipit-source-id: e5ea2e06febde892d28c9dc59dc2920d4033bb01
Summary: Separates the polyfills used for node.js from the configuration of `babel-register`, to make pretransforming packager before invoking it easier.
Reviewed By: cpojer
Differential Revision: D4978047
fbshipit-source-id: 45d3d49d0a714a8257be8d244a01e41b68bbce3d
Summary: Fixes support for haste packages in `ModuleGraph`. As `HasteMap` is no longer used for anything else, we can probably strip it down completely.
Reviewed By: cpojer
Differential Revision: D4967367
fbshipit-source-id: d40cbe46c1e8b05690c0a2c71955479c28607c01
Summary: deduplicates / unifies types for source maps across the code base
Reviewed By: jeanlauliac
Differential Revision: D4955924
fbshipit-source-id: 25cb71031dce835dd7d2bc1c27d6b20050906e81
Summary:
Two things in there:
* Using `base64` was kinda broken, as it can contain slashes, invalid in file names. It would still work however because it would just create a second level folder when doing `mkdirp`. Still I think it's better to fix that correctness.
* Include the UID in the hash, so that different users have different folders for sure, and that we reduce potiential permissions issues. `tmpdir()` already returns a folder that's user-specific on OS X, but this is not a guarantee on all platforms.
Reviewed By: cpojer
Differential Revision: D4969856
fbshipit-source-id: 4a9be35104ac9698edf2c84c58d395ee171ce2a8
Summary: Oftentimes packager stalls for several seconds after requiring a bundle, because it's busy cleaning up old cache files. We shouldn't do that when, for example, we just restarted packager. This changeset stores the micro-timestamp of the last collection so that we don't waste time next time.
Reviewed By: cpojer
Differential Revision: D4969832
fbshipit-source-id: 3aa0495b18d5a8efa1b8f87dbf7a40b0673fd9fd
Summary: Adds a stronger type for the inline plugin to help with future refactors
Reviewed By: jeanlauliac
Differential Revision: D4945842
fbshipit-source-id: f9bde75bf47271d80d2420d985a6a5609cee8952
Summary: Changes the contract for `getTransformOptions` so that it has to return a promises. That allows call sites to get rid of special logic / wrapping.
Reviewed By: jeanlauliac
Differential Revision: D4905959
fbshipit-source-id: c7d434c0766984e25987de1d769594e7c922d691
Summary: It seems we don't need that one much anymore, as we have a FS-based cache for files, and removing the last callsites doesn't really change perf, as we have to read these files eventually (plus some of these are read from `HasteMap`, that I believe is already dead code).
Reviewed By: davidaurelio
Differential Revision: D4884220
fbshipit-source-id: 4cf59f16a6f0bdf275abe81e9de2f34816866bae
Summary:
Don't build js bundles for debug simulator build configurations
Motivation:
We have more than one debug build configuration that we run in the simulator, and none of them are exactly named 'Debug'. We want to establish the convention that any simulator build configuration containing the word 'Debug' will not build the react js bundles. We believe this is less intrusive to the developer.
Closes https://github.com/facebook/react-native/pull/13472
Differential Revision: D4890622
Pulled By: javache
fbshipit-source-id: 4c809551f64ad575335416de28887a90b0756de1
Summary:
Note: if this changeset causes some breakage, consider disabling rather than reverting. To disable, the call to `_preprocessPotentialDependencies` in `ResolutionRequest` can be removed.
It's a bit of an experiment. I couldn't see any particular regression caused by this, but I could see net improvement of the global cache performance, as it unlock much, much stronger batching: indeed, instead of discovering dependencies progressively, we synchronously figure out the list of all potential modules of a bundle, and kick-off cache fetching and/or transformations. So when it comes to fetching from the global cache, it'll do less requests, and each requests will ask for considerably more keys at a time.
Potential problem caused by this changeset: if a module's dependencies completely changed, then the first time we try to build the bundle it'll start transforming modules that we probably don't care at all anymore, spending precious CPU time for nothing. I've been thinking about it and I cannot see such a case happening much often. Even if it happens, it should not cause any bug or corruption, it would just take additional time.
Other potential problem: that this new code doesn't handle some types of edge cases. It's quite hard to figure out what could possibly break in the `ResolutionRequest` code (and I think it would benefit from a larger refactor). We do have a good test coverage for `DependencyGraph` and it seems to work smoothly, so I'm relatively confident we're not breaking edge cases.
Reviewed By: davidaurelio
Differential Revision: D4875467
fbshipit-source-id: 2dfcc755bec638d3d1c47862ec1de5220953e812
Summary: The original code wasn't very readable. Better with this change, I think.
Reviewed By: davidaurelio
Differential Revision: D4851335
fbshipit-source-id: 210309d4b727aff58bea48d0ab324256234cd941
Summary:
There is a bug with IP detection on Macs that have (1) multiple ethernet interfaces, (2) are using the second or third interface as their primary connection and (3) have extra loopback IPs configured (for example when running OpenVPN, running some Docker configurations or other more exotic applications). In those cases, automatic IP detection will falsely detect the IP as something like `127.94.0.1`, which will make it impossible to run debug builds from a physical device.
The underlying network configurations looks something similar to this (irrelevant parts omitted):
```
lo0: flags=8049<UP,LOOPBACK,RUNNING,MULTICAST> mtu 16384
options=1203<RXCSUM,TXCSUM,TXSTATUS,SW_TIMESTAMP>
inet 127.0.0.1 netmask 0xff000000
inet 127.94.0.1 netmask 0xff000000
nd6 options=201<PERFORMNUD,DAD>
en0: flags=8863<UP,BROADCAST,SMART,RUNNING,SIMPLEX,MULTICAST> mtu 1500
options=10b<RXCSUM,TXCSUM,VLAN_HWTAGGING,AV>
ether xx:xx:xx:xx:xx:xx
nd6 options=201<PERFORMNUD,DAD>
media: autoselect
Closes https://github.com/facebook/react-native/pull/13415
Differential Revision: D4859654
Pulled By: javache
fbshipit-source-id: 3c5d06201c48a2dfe1c274ff433423e5d7f2cded
Summary:
When running an app on a real iPhone in debug mode, I occasionally get issues when trying to load the js bundle and I suspect the issue is with xip.io (and it doesn't look like I'm the only one: https://github.com/facebook/react-native/issues/12786https://github.com/facebook/react-native/issues/9688#issuecomment-272591839). So I've added the ability to optionally disable the use of xip.io if an env variable `DISABLE_XIP` is set.
Add `export DISABLE_XIP=true` to the `Bundle React Native code and images` build phase. Run the app on a real iPhone and ensure that it can load the JS bundle from the host computers IP.
Closes https://github.com/facebook/react-native/pull/13326
Differential Revision: D4855719
Pulled By: ericvicenti
fbshipit-source-id: cb2e91291acadaa78ea302800b55c2e5388f6380
Summary: This removes the call to `HasteFS#matchFiles()`, that has linear complexity. Instead, we index all the files by directory from `HasteFS` once loaded, a linear operation. Then, we can filter files from a particular directory much quicker.
Reviewed By: davidaurelio
Differential Revision: D4826721
fbshipit-source-id: c31a0ed9a354dbc7f2dcd56179b859e491faa16c
Summary:
One of my changeset broke the "ModuleGraph" code without warning earlier because we are using `any`, that equivalent to having no typing at all. This changeset fixes the types so that `ResolutionRequest` is exactly what it actually is: a class usable for any `Module`-looking class, including the normal one, and the "ModuleGraph" one used for Buck builds. That way, the ModuleGraph's `Module` is typechecked against `Moduleish`.
Concretely this change mostly migrates the `Module` to its generic parameter counterpart `TModule` inside `ResolutionRequest`.
Reviewed By: kentaromiura
Differential Revision: D4826256
fbshipit-source-id: fcd7ca08ac6c35e4e9ca983e2aab260e352bcb4e
Summary: Adapts mocked / duplicated functionality from `node-haste` to match the new synchronous return types in the original.
Reviewed By: jeanlauliac
Differential Revision: D4819137
fbshipit-source-id: 183316adc3fae161ad9999bf72bccb8218ef8941
Summary:
Follow up to #13248 for iOS.
This pull request allows changing the default path to `cli` when running from Xcode environment. That is especially useful when debugging from a different location or... running `Haul`.
Set `export CLI_PATH=./node_modules/react-native/local-cli/cli.js` and run. Should use new path provided.
Closes https://github.com/facebook/react-native/pull/13264
Differential Revision: D4819059
Pulled By: ericvicenti
fbshipit-source-id: 4a6241823c0bc8f1fa16869bc872bdbe04df510f
Summary:
This PR depends on #13172
Packager events are mostly logged through the TerminalReporter by default (#13172 makes this configurable). But there are a few things that aren't passed through TerminalReporter.
- We [log a banner with some information about the port and what's going on](8c7b32d5f1/local-cli/server/server.js (L22-L32))
- Also [a message about looking for JS files](8c7b32d5f1/local-cli/server/server.js (L34-L38)) (not sure what that is for / if it is useful beyond telling the user what directory root they started the packager in, but that's another thing).
- If the packager fails to start, then [we log an error message](8c7b32d5f1/local-cli/server/server.js (L41-L61)).
This pull request changes those log messages to be handled by TerminalReporter. I tri
Closes https://github.com/facebook/react-native/pull/13209
Differential Revision: D4809759
Pulled By: davidaurelio
fbshipit-source-id: 2c427ec0c1accaf54bf6b2d1da882cd6bfaa7829
Summary: At FB we log errors into the error infra, and these errors are not actionnable for end users, so let's reduce the noise generated on the terminal. In the OSS case, people can simply add a handler in a TerminalReporter decorator, the same way we do internally (anyhow, I do not know of anyone using the global cache in OSS for now).
Reviewed By: davidaurelio
Differential Revision: D4762858
fbshipit-source-id: 880c02e175ae551df11b7ce273acc318222c46bf
Summary: Some more synchronicity, one step at a time.
Reviewed By: davidaurelio
Differential Revision: D4756542
fbshipit-source-id: 0c56dbca61b3da764aa8d28e29c0e20b54de091e
Summary: We want any exception thrown by `_getHasteName` to be captured by the promise instead of breaking the outer stack.
Reviewed By: davidaurelio
Differential Revision: D4754825
fbshipit-source-id: 173c7c8867da73efb198ed3159704d6fd0e7b87d
Summary: In case the sync function throws, it wouldn't be handled through the promise anymore, that is not what we want. So we revert that in this changeset.
Reviewed By: davidaurelio
Differential Revision: D4754740
fbshipit-source-id: 4da360f4b629bbdf9cd284389060429cc9259c2c
Summary: Finally adding some unit test to increase confidence in the correctness of that piece of code.
Reviewed By: davidaurelio
Differential Revision: D4721543
fbshipit-source-id: 56776290d61f2b51c69d7eeae09663e3bc892b50
Summary: When requiring a module that has previously errored, the implementation of `require` only used the numerical module ID. In this diff, we enable usage of the verbose module name if present.
Reviewed By: bestander
Differential Revision: D4737723
fbshipit-source-id: 1c2d3906435a637f3e440e57f904489d84495bd2
Summary: The logic of the `inline` babel transform regarded identifiers as global if no binding exists for them. We extend that logic to also accept flow-declared variables.
Reviewed By: arcanis
Differential Revision: D4737620
fbshipit-source-id: e71cfdf77c7b7751265cfa4412430b4f29e9e853
Summary: Fixes the messed-up indentation of `polyfills/require.js`. Over half of the lines were indented with an odd number of spaces.
Reviewed By: arcanis, bestander
Differential Revision: D4737435
fbshipit-source-id: a5b9baf0a27f236a4d3d6b6c1c5a92f52859f62c
Summary:
This changeset adds a test that verifies that the duplicate modules use case is broken. The goal is to acknowledge the problem for now, and when we update `jest-haste`, we'll simply fix what needs to be fixed in that test.
See also, https://github.com/facebook/jest/pull/3107
Reviewed By: davidaurelio
Differential Revision: D4673431
fbshipit-source-id: 05e09bdf61a2eddf2e9d1e32a33d32065c9051f1
Summary:
The breakage fixed by changeset [1] could have been identified earlier if we had typing on `attachHMRServer`, so I spent some time on that. This has revealed in turn a few functions across the codebase that were incorrectly typed, and that are now fixed.
[1] packager: attachHMRServer.js: fix callsite of Server#getModuleForPath()
Reviewed By: davidaurelio
Differential Revision: D4706241
fbshipit-source-id: fc4285245921ae45d5781a47d626fc0559dba998
Summary: The problem with `bundleOpts` is that it discards Flow typing, so it prevents reinforcing the integration of `Bundler` into `Server`. This changeset removes the `bundleOpts` to solve that issues. Instead, it makes the options explicit so that there is less uncertaintly. I love making options explicit, because they force callsites to take a consicious decision about what is really needed, making them more robust. They also expose oddities that probably needs refatoring, for example having a `resolutionRequest` in the bundle options does not seem correct, it should be an implementation details. Likewise, `onProgress` should probably be exposed differently, as it does not affect the content of the bundle itself.
Reviewed By: davidaurelio
Differential Revision: D4697729
fbshipit-source-id: d543870ba024e7588c10b101fa51429c77cc5ddc
Summary: Also remove the unnecessary await of the resolver, because `_bundler.bundle()` already does that.
Reviewed By: davidaurelio
Differential Revision: D4689448
fbshipit-source-id: 3b4fd73b1368f8b00c6eb7483e751387d9856ce9
Summary: The function giving the worker count was duplicated, let's just pass it down from a central place, the Bundler. Also, I simplified the function to use a simple formula rather than arbitrary ranges (it's still arbitrary, just a tad bit less :D ).
Reviewed By: davidaurelio
Differential Revision: D4689366
fbshipit-source-id: fe5b349396f1a07858f4f80ccaa63c375122fac8
Summary: Not having default everywhere (keeping them at the top level instead) makes for a code that is easier to understand, and more robust as different pieces of code cannot default to different values. This changeset also unifies the option types (ex. `platform`).
Reviewed By: cpojer
Differential Revision: D4688882
fbshipit-source-id: b5f407601386336f937a0ac1f68c666acc89dfd8
Summary:
Absolute imports on Windows were broken, I'm not 100% sure when this happens but when I tested Exponent on Windows which uses `rn-cli.config.js` with
```js
getTransformOptions() {
return {
reactNativePath: path.resolve('./node_modules/react-native'),
reactPath: path.resolve('./node_modules/react'),
};
}
```
it seemed to use absolute paths for these modules.
I also tested absolute paths in node repl and it does work for absolute paths of different formats. `C:/root/test.js`, `/root/test.js`, `C:\root\test.js` all do resolve properly to the same module.
To fix this I resolve the absolute path using `path.resolve` on Windows. Noop on other platforms to avoid the overhead since it's not necessary.
**Test plan**
- Tested that it fixed the bug I had when running Exponent on Windows.
- Updated the absolute path test to use forward slashes since this is what happens in practice when using `getTransformOptions`. We can't test all cases on linux since adding the drive letter au
Closes https://github.com/facebook/react-native/pull/12530
Differential Revision: D4634699
Pulled By: jeanlauliac
fbshipit-source-id: 0cf6528069b79cba2e0f79f48f5a524d59b7091e
Summary:
Similar to https://github.com/facebook/jest/pull/2877, this introduces an optional config `HasteImpl` of type `{getHasteName(filePath: string): (string|void)}` that returns the haste name for a module at filePath if it is a haste module or undefined otherwise.
This allows us to inject a custom implementation of haste's module id resolution rather than only relying on `providesModule` annotations
Reviewed By: davidaurelio
Differential Revision: D4589372
fbshipit-source-id: 4d1983dfbf09c9d67faf725e86ae86ab42433b7d
Summary:
Moves stack trace symbolication to a worker process.
The worker process is spawned laziliy, and is treated as an exclusive resource (requests are queued).
This helps keeping the server process responsive when symbolicating.
Reviewed By: cpojer
Differential Revision: D4602722
fbshipit-source-id: 5da97e53afd9a1ab981c5ba4b02a7d1d869dee71
Summary:
These are not modules and don't need a `providesModule` annotation.
`sed -i -e '/providesModule/d' packager/src/Resolver/polyfills/*.js`
Reviewed By: cpojer
Differential Revision: D4605374
fbshipit-source-id: 5045a9664bc105dab15936f408d373da8d9322fe
Summary:
HTTP request URLs don’t include protocol, host and port. Stack frames URLs, on the other hand, contain full URLs. These full URLs are used to get the correct bundle to build the source map from.
The method that creates option objects from URLs therefore now discards leading protocol, host and port to ensure that cached bundles can be reused for symbolication rather than triggering rebuilds.
Reviewed By: jeanlauliac, cpojer
Differential Revision: D4598077
fbshipit-source-id: 262df187bcdf7099011371e8b55ae692c6e1a942
Summary:
I extracted all the dependencies (using jest-haste-map) and copied them from the package.json in react-native to RNP. There is some duplication here for now but we can later go back and remove the duplicated dependencies from react-native that aren't needed there.
I also removed a mock file that hasn't been in use for a long time.
Reviewed By: jeanlauliac
Differential Revision: D4598155
fbshipit-source-id: 850b6dfa6fc2eec138ebdd7208cd34bee21f7927
Summary:
The first time I tried to commit this changeset, it was causing many new packages to be installed, because the dependency would depend on newer versions that what we have installed. So, I had made a diff so upgrade all the babel packages. Unfortunately this caused some problem as the newer versions of Babel are more strict on some syntaxes. Of course, these have to be addressed, but I don't want this changeset to be coupled with Babel upgrades and the issues that arise from it.
So instead, I decided to install the slightly older version of the async-to-generator module. At first I tried with just doing:
yarn add babel-plugin-transform-async-to-generator@6.16.0
But, `yarn` is stubborn: because this module depends on a caret version of `babel-helper-remap-async-to-generator`, it installs the very last version of it, that itself needs more recent versions of other Babel modules. So, instead, I add to install a slightly older version of the dependency manually, then then the plugin:
yarn add babel-helper-remap-async-to-generator@6.16.0
yarn add babel-plugin-transform-async-to-generator@6.16.0
This allows us to have a `yarn.lock` with only a minimal amount of changes, and uncouple this change from any Babel upgrades. Because we only have a few new modules, the `node_modules` folder also stays the same, 133M, and it gives us confidence this will not cause significant startup time regressions.
Reviewed By: cpojer
Differential Revision: D4578733
fbshipit-source-id: deb0f720b895b7196aaf432adec3e56f18663940
Summary:
allow-large-files
By using async/await the code is (1) less nested, (2) more compact and (3) more robust (no exceptions running away, and much less risks of forgetting to call the callback/resolve, or mistakenly calling it twice). I now tend to think we could switch to it for all the callsites that are not in a perf-critical path.
I switched from 'request' to 'node-fetch' because 'request' has an annoying callback with 2 arguments. So it's simpler to use an interface that's (1) already returning a Promise and (2) that is becoming standard.
This changeset was a way for me to start experiment with introducing async/await in packager codebase, and it looks pretty good so far.
Reviewed By: cpojer
Differential Revision: D4559167
fbshipit-source-id: 89a328c5766c2ba890e9d0e67a81a38dac6cfc73
Summary: I figured out that the uncaught rejection that happens on transform error originated from a "fork" along the chain, where we both "then" on a promise, and return that promise. In that case Node.js, legitimately I think, identifies this as an uncaught rejection case. One solution, in this changeset, is to do the side-effects as part of the promise chain instead of "forking". Another option would be to add an explicit error handler to the additional "then", but it seems we don't have to handle that case here.
Reviewed By: davidaurelio
Differential Revision: D4515592
fbshipit-source-id: 17d813cfdbc76685b6273b8d94e97d948fd68674
Summary:
This moves the `src` directory one level up and removes the `react-packager` folder. Personally, I always disliked this indirection. I'm reorganizing some things in RNP, so this seems to make sense.
Not sure if I forgot to update any paths. Can anyone advice if there are more places that need change?
Reviewed By: jeanlauliac
Differential Revision: D4487867
fbshipit-source-id: d63f9c79d6238300df9632d2e6a4e6a4196d5ccb
Summary: We can simplify as there's only one option, and it seems to me we can also make it required, as otherwise the `JSTransformer` is essentially doing nothing. I don't believe we still have use cases where no transform happens at all, even in OSS?
Reviewed By: davidaurelio
Differential Revision: D4481723
fbshipit-source-id: b2e3830b206d56242d298ff3a7b5f4587ecfd29a
Summary: It's not used from any callsite as far as I can see.
Reviewed By: davidaurelio
Differential Revision: D4481708
fbshipit-source-id: 2c503fb7ef20f9370a950c315832f3ace4709739
Summary: it's not used from any callsite as far as I can see.
Reviewed By: davidaurelio
Differential Revision: D4481699
fbshipit-source-id: 4cf63ef7953d6cfc58e7ef4f22ecb99bf51e76a0
Summary: It's not used in any callsite.
Reviewed By: cpojer
Differential Revision: D4481683
fbshipit-source-id: 3fa55693f5f56b4fb6c455ad77d7780f69be81a9
Summary: The idea is to make it easier to interact with tools consuming the packager's output. For example, Nuclide. Do you think that'd work well?
Reviewed By: davidaurelio
Differential Revision: D4482041
fbshipit-source-id: 6c64d7963195a4d786ed8902640f9e9f279f5f83
David Aurelio
Gabe Levi
Jean Lauliac
Dominic Gannaway
Sebastian McKenzie
Janic Duplessis
Kevin Old
Andres Suarez
cpojer
Lukas Piatkowski
Karol Kuczmarski
Ashok Menon
Alex Kring
Christoph Pojer
Hilke Heremans
Jonathan Kim
Mike Grabowski
Tim Yung
Brent Vatne
Pieter De Baets
Lee Byron
Marc Horowitz
Kevin Cooper
Liu Zhanhong
Alexey Lang
sonaye
Bhuwan Khattar
Jhen
Alexey Kureev
