Summary:
The packager dependencies CLI command always operates on --dev=true today. This means any tooling that needs to get the production dependencies (--dev=false) will always get the dev-mode list instead. For instance:
```
if (__DEV__) {
require('Foobar');
}
```
Previously, `Foobar.js` will always be listed in the CLI output. With this change, setting `--dev false` option will correctly skip `Foobar.js` in the output.
Reviewed By: cpojer
Differential Revision: D5163184
fbshipit-source-id: 203221ee5d6ecb7df575442f12f6c4c489bfbd46
Summary: This fixes a longstanding bug that happens when 2 bundles with the same entry path but different options (dev, minify, etc.) get mixed up in the reporting. To prevent that we just use a unique build ID for each bundle that the Server handles separately.
Reviewed By: cpojer
Differential Revision: D5147049
fbshipit-source-id: da5c9cfe8c6a5d888b5be737947800d213081d86
Summary:
This changeset moves the creation of the transform cache at the top-level of the bundler so that:
* we can use alternative folders, such as the project path itself, that I think will be more robust especially for OSS;
* we can disable the cache completely, that is useful in some cases (for example, the script that fills the global cache).
The reasons I believe a local project path is more robust are:
* there are less likely conflicts between different users and different projects on a single machine;
* the cache is de facto cleaned up if you clone a fresh copy of a project, something I think is desirable;
* some people have been reporting that `tmpDir` just returns nothing;
* finally, it prevents another user from writing malicious transformed code in the cache into the shared temp dir—only people with write access to the project have write access to the cache, that is consistent.
Reviewed By: davidaurelio
Differential Revision: D5113121
fbshipit-source-id: 74392733a0be306a7119516d7905fc43cd8c778e
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: This makes it easier to verify correctness when adding new config/args.
Reviewed By: davidaurelio
Differential Revision: D5069537
fbshipit-source-id: 4d8058851900b23163d0f2744e91dd14dfcdd461
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: 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: `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 module is not used anymore, remove it and its dependency `joi`.
Reviewed By: cpojer
Differential Revision: D5028909
fbshipit-source-id: 90b9b156fbfe642cce93a530faf8ce91c5b848f5
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:
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:
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:
`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: deduplicates / unifies types for source maps across the code base
Reviewed By: jeanlauliac
Differential Revision: D4955924
fbshipit-source-id: 25cb71031dce835dd7d2bc1c27d6b20050906e81
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:
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:
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:
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:
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 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
Kevin Gozali
Jean Lauliac
Christoph Pojer
Gabe Levi
David Aurelio
Janic Duplessis
Lukas Piatkowski
Ashok Menon
Tim Yung
Bhuwan Khattar