Summary:
'source' is not available in all shells on Linux (e.g. dash) and will silently fail launchPackager.command when called from runAndroid.js.
react-native run-android will thus silently fail to start the developement server ('JS server').
What existing problem does the pull request solve?
When running "react-native run-android" on the below reasonably vanilla Ubuntu system, the development server / packager script fails to start. It fails because sh defaults to dash (not bash) which doesn't know the command 'source'. dot (.) does the same as source, but works in all shells.
$ uname -a
Linux dallas 4.8.0-52-generic #55~16.04.1-Ubuntu SMP Fri Apr 28 14:36:29 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
$ which sh
/bin/sh
$ readlink -f /bin/sh
/bin/dash
react-native-cli: 2.0.1
react-native: 0.44.0
$ ps aux | grep packager
(nothing)
ps aux | grep packager
sh /home/xxx/code/react-native/AwesomeProject/node_modules/react-native/packager/launchPackager.command
node /home/xxx/code/react-native/AwesomeProject/node_modules/react-native/packager/../local-cli/cli.js start
Closes https://github.com/facebook/react-native/pull/14040
Differential Revision: D5096298
Pulled By: hramos
fbshipit-source-id: 88466e802c9bc4358840391edb37e153f0a6b1f7
Summary: Changes the `sourceMappingURL` comment from multi line to single line.
Reviewed By: johnislarry
Differential Revision: D5094791
fbshipit-source-id: f8fa2efd7f173290cac985564c06b4a118a80aae
Summary: separates modules of the dependency graph from the generated require calls to kick of the app. This is required to make RAM bundle generation work properly'
Reviewed By: jeanlauliac
Differential Revision: D5094635
fbshipit-source-id: fca69a3e2d9b030cdc4d4405c2b5e795b0d55f87
Summary: Releases the dependency map of modules after running the factory in release mode in order to save memory.
Reviewed By: cpojer
Differential Revision: D5086693
fbshipit-source-id: 68c57a2f98182ed1a732e1336e6d4fe5ce27abc3
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
cpojer
Sebastian Bargmann
David Aurelio
Jean Lauliac
Gabe Levi
Dominic Gannaway
Sebastian McKenzie