Summary:
Argument was no longer used and is causing lint errors.
Dropping it simplifies the debounce function as well.
Reviewed By: davidaurelio
Differential Revision: D6999567
fbshipit-source-id: 8856ed40feb0fa23155c9d18e20c43550aac876b
Summary: Continue cleaning up the resolution algo. By removing the class completely as well as the `TModule` type, and providing options via the `context`, we'll be able to identify clearly the inputs of the resolution algorithm, and reuse it eventually for Jest. This changeset extract one more function in this fashion. Next steps: extract the rest of the stateful code into functions, or into `ResolutionRequest`.
Reviewed By: davidaurelio
Differential Revision: D6937569
fbshipit-source-id: bab0bcd280be32a11f23174b0591dd0cc7dee7ab
Summary:
**Summary**
The cache dir name now starts with `metro-bundler-cache-` but the message still refers to `react-`.
This just wasted a bit of my time before I realized and that's not the first time this happens either.
Also I don't have `$TMPDIR` defined on vanilla ubuntu, I think using that instead of `/tmp` creates more confusion than helps.
**Test plan**
Just a change in a string constant.
I signed the CLA.
Closes https://github.com/facebook/metro/pull/131
Differential Revision: D6923347
Pulled By: rafeca
fbshipit-source-id: 4e816e5d0c75b0d2ee5ddf4d92d2d7397011d4e4
Summary:
I think the default value for the `--source-map` option is true, although it's not defined that way explicitly. Regardless, when it's explicitly set to `false` I would expect Metro to at least listen to that. Currently that's not the case. This diff attempts to improve that situation.
Note that it's still not perfect since there are more places where sourcemaps are still being touched when this flag is `false`. It turns out adding a proper flag for this leads to a lot of cascading type problems and so I ended up with this fix instead.
Reviewed By: rafeca
Differential Revision: D6884423
fbshipit-source-id: 7fe10045d9d9f73c7841588627ba41b14c2e412b
Summary: Don't bother to work on the source map if it won't be used anyways
Reviewed By: rafeca
Differential Revision: D6884244
fbshipit-source-id: ac014bf982e33c669a1e79f16dd06149e758a797
Summary:
This function was working on a source map object inline and then returning the same reference.
For clarity it's better if it either clones the whole thing (that would be a waste) or not return the reference at all. Also renamed the function (but not the file) for clarity.
Dropped the unused check and updated callsites instead.
Reviewed By: mjesun
Differential Revision: D6884241
fbshipit-source-id: 33aa7a1ad7096510903bbb74dd6b9c675b81aff5
Summary: Don't bother to work on the source map if it won't be used anyways
Reviewed By: mjesun
Differential Revision: D6884247
fbshipit-source-id: 811409313e7ac1a7255ddd0eed665ed92020241e
Summary: Don't bother to work on the source map if it won't be used anyways
Reviewed By: mjesun
Differential Revision: D6884243
fbshipit-source-id: 483664ad32a72a8e6e3de3e0cc224b3a6ea67d46
Summary: This prevents unnecessary JSON.stringify/JSON.parse when the operation is noop anyways.
Reviewed By: mjesun
Differential Revision: D6884246
fbshipit-source-id: 3370caac1e01cb79f86af95f0dd06285150c5d95
Summary:
An Ast consists of `Node` elements and the one Babel uses is specific to Babel (incompatible with Astree spec by default) so `BabelNode` seems more appropriate.
Keeping the other `ast` names for now to reduce the footprint of this diff.
Also makes sure `babel-core` properly exports the same node type as what the api returns making fixes to this type in the future easier to implement.
Reviewed By: davidaurelio
Differential Revision: D6834291
fbshipit-source-id: 4cb0eb2ee280a4988071b8412539edfad12cf56f
Summary: This test can be simplified.
Reviewed By: davidaurelio
Differential Revision: D6834289
fbshipit-source-id: 23072230ce844c39b5fba30cfe7ad51e5a879bed
Summary:
This bridge pulls in the various babel things and exports them again.
In a future update this bridge will also pull in babel 7 versions of the same thing and an env var will determine which sources to use so you can switch between using babel 6 or 7 through ENV.
Reviewed By: davidaurelio
Differential Revision: D6844862
fbshipit-source-id: 610a60eaf7bf368eddcd015e37f228651e2fd78b
Summary: The type was not imported and this escaped the attention of Flow somehow.
Reviewed By: davidaurelio
Differential Revision: D6834292
fbshipit-source-id: 8a9b5f2ac56fca2d0ff298b52f54935327eb2686
Summary: The type was not imported and this escaped the attention of Flow somehow.
Reviewed By: davidaurelio
Differential Revision: D6834298
fbshipit-source-id: 96f754d5aae9fda661cb18bbbb30ccfd033598a1
Summary: Releasing a new version of metro with an executable CLI
Reviewed By: davidaurelio
Differential Revision: D6808207
fbshipit-source-id: 68f6522924ea8ad7b6f9aaa3e952ebcf23d2cf8b
Summary: To be able to run the metro CLI directly (for example via `yarn run`) we need to add the shebang pointing to the nodejs binary
Reviewed By: davidaurelio
Differential Revision: D6808095
fbshipit-source-id: 89628c07a92266432a404b6bc26eaa412d7859b9
Summary: Tries to adress https://github.com/facebook/metro/issues/65. We need a reasonnable workaround to support modules like `moment.js` that do dynamic requires but only in some cases. By replacing the call by a function that throws, we move the exception at runtime instead of happening at compile time. We don't want to do that for non-node_modules file because they are fixable directly, while `node_modules` are not fixable by people and they get completely blocked by the error at compile time.
Reviewed By: rafeca
Differential Revision: D6736989
fbshipit-source-id: a6e1fd9b56fa83907400884efd8f8594018b7c37
Summary:
Depends on #124.
---
**Summary**
Metro reports errors using a JSON payload that has an `errors` array. Each item in this array has a `description` field. For transform errors, this field was set using the value in `error.description` -- however, JS Error objects only have a `message` field. (Grepping the Metro code, no errors (except in one test) ever get a `description` field.) This commit uses `error.message` instead of `error.description` when creating JSON payloads.
```
$ git grep description -- 'packages/**/*.js'
packages/metro/src/JSTransformer/__tests__/Transformer-test.js: babelError.description = message;
packages/metro/src/lib/formatBundlingError.js: description: string,
packages/metro/src/lib/formatBundlingError.js:): {type: string, message: string, errors: Array<{description: string}>} {
packages/metro/src/lib/formatBundlingError.js: errors: [{description: message}],
packages/metro/src/lib/formatBundlingError.js: description: error.message,
packages/metro/src/node-haste/__tests__/Module-test.js: description: "A require('foo') story",
```
**Test Plan**
Added a unit test to check that the description field is set for transform errors (with the delta bundler).
Also in a test RN app, inspected the error payload that is received by RN when there's a syntax error with HMR turned on and verified that `data.body.errors[0].description` was set.
Closes https://github.com/facebook/metro/pull/125
Differential Revision: D6730671
Pulled By: rafeca
fbshipit-source-id: 58311462db9223d65580d77748203d8ea0ea1ac7
Summary:
**Summary**
With RN 0.52, when there was a redbox due to a syntax error in a source file (with regular, non-delta bundler), the redbox would say just "No message provided". The JSON that Metro sent to RN did not include a "message" field because `JSON.stringify(error)` does not include `message`.
**Test Plan**
Add a syntax error to one of the files in RNTester's JS and load the RNTester app (from RN master). See that the redbox now says there was a transform error with the syntax error's location.
Also tested adding a syntax error with HMR enabled and saw that the error `message` field was set in the payload as expected.
Also added a Jest test to Server-test.js.
Closes https://github.com/facebook/metro/pull/124
Differential Revision: D6728310
Pulled By: rafeca
fbshipit-source-id: 9ee3c113767d8c2849bcaac0cb8a9cfa8f2760a6
Summary:
**Summary**
Metro used to have support for "asset plugins", which allowed developers to specify arbitrary JS modules that could export a function for adding more fields to asset data objects. Some of this functionality was removed in the delta bundler work -- this PR adds it back.
**Test plan**
Made existing unit tests pass and added unit tests to test asset plugin behavior. Also tested E2E in a React Native project by adding `assetPlugin=/path/to/pluginModule` to a JS bundle URL and ensuring that the plugin ran.
Closes https://github.com/facebook/metro/pull/118
Differential Revision: D6711094
Pulled By: rafeca
fbshipit-source-id: f42c54cfd11bac5103194f85083084eef25fa3cd
Summary: This changeset tweaks the prelude so as to avoid overriding `process.env` if it already exists, and add tests to verify this behavior. This is useful when a bundle is loaded in a context that already provides a global `process` somehow, ex. Electron/Node.js. We still do want to override `NODE_ENV` in that case so as to have consistency with constant inlining.
Reviewed By: rafeca
Differential Revision: D6702441
fbshipit-source-id: 69dd9ba2303a43db151cbe1877f01e38d45b05b9
Summary: Next step would be to move the error handling out of this module so that we don't depend on `TModule` at all anymore. Once this is done, the module can be extracted as `metro-resolve`, and we can potentially reuse it for jest, etc.
Reviewed By: davidaurelio
Differential Revision: D6660540
fbshipit-source-id: dd0612bec6b526f9ab52cc2e162b6977e2b1670f
Summary:
**Summary**
Metro crashed with "Allocation failed - JavaScript heap out of memory" when trying to build a project with a lot of assets. (Repro: https://github.com/fson/metro-out-of-memory, `yarn start` and open the app.)
By looking at a heap snapshot, I found out the reason was that `node-haste` was reading the contents of asset files and parsing them in search of the `providesModule` docblock field, although it should only do this for source files. I fixed this by checking that `isHaste()` evaluates to `true` before attempting to read the docblock. (`AssetModule` always returns `false` for `isHaste()`.)
**Test plan**
* Ran the [repro](https://github.com/fson/metro-out-of-memory) before the fix. It crashed with `FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory`
* Heap snapshot: <img width="1440" alt="screen shot 2018-01-03 at 17 31 53" src="https://user-images.githubusercontent.com/497214/34571809-33ee8dfc-f178-11e7-93f9-3fd80e62c188.png">
* After implementing the fix, ensured that the process didn't crash anymore and the heap didn't grow significantly during the build:
Closes https://github.com/facebook/metro/pull/112
Reviewed By: jeanlauliac
Differential Revision: D6660719
Pulled By: rafeca
fbshipit-source-id: d4d8d4445e75274c65f6a52027d878041f1c9a33
Summary:
As reported in https://github.com/facebook/metro/issues/109, the sourcemaps generated by metro are including the binary data of all the assets, which adds a lot of overhead and is not useful at all.
This commit removes the assets contents from the sourcemaps to fix https://github.com/facebook/metro/issues/109
Reviewed By: jeanlauliac
Differential Revision: D6649741
fbshipit-source-id: 8b09d429a1aa8d487557c22440bfa73ae55d03bd
Summary: Finally we can extract this function out, that remove any remaining dependency on the `TModule` generics and the like. Next steps are to push Metro-specific concepts out of the `resolveDependency` function and out of the module completely, after which the module will be ready to have its own unit tests, and be split into a `metro-resolve`.
Reviewed By: cpojer
Differential Revision: D6645333
fbshipit-source-id: a7915d646f888c29b410bf211f2e2bc4ec157676
Summary: This changeset alters the Haste resolution logic to be in sync with the new refactored pieces (resolveFile, etc.) Next steps involve extracting this function outside of the class so as to reduce the surface API and promote composability.
Reviewed By: cpojer
Differential Revision: D6645302
fbshipit-source-id: 6ee00fb52025cc0d332e57f837b46e8323faf6f4
Summary: In a further diff, I'll add cases where this error may be thrown at a higher level than the `resolveFileOrDir` function. So we need to move the handler and wrapping logic up the stack, that doesn't have any difference in practice.
Reviewed By: cpojer
Differential Revision: D6642475
fbshipit-source-id: 6b746386a170bfc167a9b50d9786fbb98c920c6d
Summary:
This is part of the efforts to clean up and extract the resolution logic. In this diff, the haste resolution is moved as part of the main resolution function rather than having 2 separate top-level functions. The reason for doing this is that logic is duplicated otherwise, making the already complex code harder to follow still. An example of duplicated logic is the call to `isRelativeImport`, that is done both in ResolutionRequest and ModuleResolution's node resolver. In that case, we never want to use Haste. Another duplication is the redirect of requires of package/haste names. With this changeset it is done in a single place at the beginning of the algo.
This changeset causes slight changes in behaviors. For example, consider `require('Foo/')`. A lookup of `Foo` would be done in the package `browser` field for a potential redirect. With this new code, we'll only look for `Foo/` in the redirect map. My opinion is that this is for the better, as this uniformize the way it works with Node.js `node_module` packages, making resolution more predictable. We should, additionally, actively strive to get rid of trailing slashes anywhere they might be in RN as they bring no apparent technical feature, but more confusion (I might be missing context, naturally).
Next steps will be to clean up `_resolveHasteDependency` completely, removing the callsite `try..catch` in favor of saner conditionals, and enforcing Haste packages to resolve if they exist (right now if a Haste package is found but corrupted, we just continue merrily trying to resolve the module with the rest of the logic; we should hash-crash instead, same as has been done for `resolvePackage`).
Reviewed By: cpojer
Differential Revision: D6642347
fbshipit-source-id: 2f40575b35916b644f342e0267c465a89bee202c
Summary: This improves the error message by providing line/column so we can identify errors encountered in https://github.com/facebook/metro/issues/65 easier. Also provide additional fields so we can better format that later on.
Reviewed By: cpojer
Differential Revision: D6641909
fbshipit-source-id: 0372da6b2fb51010b42ab906fc40b528b1483532
Summary:
The spec doesn't say the `browser` field should actually generate a replacement for the main *file*, just that it provides an alternative main: https://github.com/defunctzombie/package-browser-field-spec#alternate-main---basic
We also don't have any test covering this behavior. Any package relying on this behavior should be fixed to have a redirection of the main *file* in addition to their `main` *field*.
This is part of an effort to cleanup the way we do redirections. Eventually this will be part of the resolution module, not `Package.js`.
Reviewed By: davidaurelio
Differential Revision: D6611894
fbshipit-source-id: 0186aa23df2a9183d895ea02b5fd3b2b7339dc76
Summary: This is one of the first step towards a generic and composable resolution algorithm. We shall get rid of `TModule` as it's an abstraction that doesn't bring us any benefit in the resolution algo, and we actually want to get rid of `Module.js` eventually. This changeset gets rid of `TModule` up to the place where I already got rid of the exception-based control flow mechanism. Next steps are to push the boundaries higher up both for error handling, and for removing `TModule`.
Reviewed By: mjesun
Differential Revision: D6603157
fbshipit-source-id: 551e9ccd93628f45452148ed40a817bdde3740ea
Summary: I want to get rid of `TModule` and stuff so that we can eventually reunite this with `jest-resolve`. This is a continuation of the cleanup I started a long time ago.
Reviewed By: davidaurelio
Differential Revision: D6601657
fbshipit-source-id: 62c1806783323ee50e9a09146c73006c721eb891
Summary:
On D6248242, runBeforeMainModule was changed to `getModulesRunBeforeMainModule`, but the entry point used by opensource to build prod bundles was not updated. We couldn't catch this since we don't use this codepath internally
Fixes https://github.com/facebook/metro/issues/73
Reviewed By: mjesun
Differential Revision: D6556097
fbshipit-source-id: 889eaf825c7c3cdebe1ca4fc9831020a4a7d56dc
Summary:
Addresses a performance regression introduced by automatic linting: Compound assignment operators are much slower than keeping assignment separate on `let` bindings in certain versions of v8.
This reverts the relevant changes and configures eslint to *disallow* these operators explicitly in `metro-source-map`.
Reviewed By: mjesun
Differential Revision: D6520485
fbshipit-source-id: 16f35f5cd691ce6b1924480cbc30fbaa1275f730
Summary:
**Summary**
`createModuleIdFactory` is already used in `metro` internally, but is currently always a fixed function.
This enables `metro.runBuild()` to be run with a custom module ID factory.
One use case: building a base bundle, on top of which other application-specific bundles could reference modules in the base. The application-specific IDs need to not conflict with those in the base bundle, and all references to modules in the base must resolve to the correct ID in the base bundle. A custom ID factory can make all this possible.
**Test plan**
<!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI. -->
Using `metro.runBuild(...)` with these changes, I was able to substitute in a custom ID factory
```javascript
const fs = require('fs')
const metro = require('metro')
const baseManifestFileContents = JSON.parse(fs.readFileSync('./baseManifest.json'))
// baseManifest looks like:
// { "modules": { ...
// "react/index.js": { "id": 12 },
// "react/cjs/react.production.min.js": { "id": 13 }, ... } }
const opts = {
dev: false,
entry: 'index.js',
out: './out.bundle',
platform: 'ios',
projectRoots: ['.', 'node_modules'],
config: {
createModuleIdFactory: createModuleIdFactory(baseManifestFileContents)
}
}
metro.runBuild(opts)
// Creates a sample custom ID factory
function createModuleIdFactory(manifestFileContents) {
return function createModuleIdFactory() {
const fileToIdMap = new Map()
let nextId = manifestFileContents ? getNextIdAfterBaseManifest(manifestFileContents) : 0
return path => {
const sourcePath = path
.replace(process.cwd() + '/node_modules/', '')
.replace(process.cwd(), '.')
// If module is in the base manifest, return its ID
if (manifestFileContents && manifestFileContents.modules[sourcePath]) {
return manifestFileContents.modules[sourcePath].id
}
// Otherwise, get it from the map or create a new ID
if (!fileToIdMap.has(path)) {
fileToIdMap.set(path, nextId)
nextId += 1
}
return fileToIdMap.get(path)
}
}
function getNextIdAfterBaseManifest(manifestFileContents) {
return Object.keys(manifestFileContents.modules).reduce((id, key) => {
if (manifestFileContents.modules[key].id > id) {
return manifestFileContents.modules[key].id
}
return id
}, 0) + 1
}
}
```
With the sample module ID factory above, the output looks like the following, where defined module IDs start at a higher number to avoid the base module IDs, but may depend on modules in the base bundle (lower numbers).
```javascript
...
__d(function(r,o,t,i,n){t.exports=r.ErrorUtils},551,[]);
__d(function(n,t,o,r,u){'use strict';var e,c=t(u[0]);e=c.now?function(){return c.now()}:function(){return Date.now()},o.exports=e},552,[553]);
...
__d(function(e,t,r,s,l){'use strict'; ...},564,[18,565,27]);
...
```
Closes https://github.com/facebook/metro/pull/100
Reviewed By: mjesun
Differential Revision: D6508351
Pulled By: rafeca
fbshipit-source-id: f2cfe5c373a6c83c8ae6c526435538633a7c9c2a
Summary:
With this, we do all the transformation and wrapping of files inside the workers, which mean faster initial builds (because parallelization and caching), and more legacy code that can be removed (see the following diff).
I've done some tests locally, and while the initial builds are slightly faster, the increase is not super substantial (the big win was in the diff were I moved the wrapping of JS files, in this diff only the assets transformation has speed up).
The most important thing that this diff enables is the possibility of doing the minification of modules also in the worker. This will mean that we can cache minified files and prod builds will get significantly faster - almost as fast as development builds (this will be relevant mainly for the opensource community).
Reviewed By: davidaurelio
Differential Revision: D6439144
fbshipit-source-id: ba2200569a668fcbe68dbfa2b3b60b3db6673326
Summary: This fixes https://github.com/facebook/metro/issues/99, which causes issues when metro tries to build files that are ignored by babel (if babel transformer ignores a file, it returns a null AST, which makes the collectDependencies logic break).
Reviewed By: mjesun
Differential Revision: D6466878
fbshipit-source-id: b5030e03775b982958a0b9204f4efccc8940ae4d
Summary: I'm working on getting CI to pass. As a first step, I'll upgrade the lerna setup to use Yarn's workspaces (when yarn is run from the Metro root) as well as upgrading Flow to the same version we use in xplat. I also copied over the Jest type definitions. This should fix all type errors for a start.
Reviewed By: davidaurelio
Differential Revision: D6361276
fbshipit-source-id: 4e8661b7d5fe4e3f6dd1e6923891bd2d23c9b4db
Summary:
<!-- Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. The two fields below are mandatory. -->
**Summary**
<!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? -->
It is currently not possible to resolve specific module imports (such as `react-native/Libraries/Image/AssetRegistry` using forward slashes as folder separators) using a custom mapping defined in `extraNodeModules` on Windows. This PR solves this issue by normalizing module names to use platform-specific folder separators before splitting the module name using `path.sep` as separators.
**Test plan**
<!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI. -->
We use `extraNodeModules` to create a mapping between `react-native` and a forked and scoped version of react-native (i.e. `scope/react-native`). If we import a specific file from react-native (such as [`react-native/Libraries/Image/AssetRegistry`](https://github.com/facebook/react-native/blob/master/local-cli/core/Constants.js), which gets referenced when importing image assets), `ModuleResolution.js` is not able to extract the first folder name on Windows. This first folder name is the name of the module (`react-native` in the previous example) and is used to query `extraNodeModules` for possible matches. It is not able to find this folder name because the module name is split using `path.sep` as a separator, which is `'\\'` on Windows. Most module names use forward slashes as folder separators. The solution is to normalize `toModuleName` before we split on `path.sep`.
Closes https://github.com/facebook/metro-bundler/pull/89
Differential Revision: D6312391
Pulled By: jeanlauliac
fbshipit-source-id: 920c52633e8c9584ecb2bdd309dc4a8516c3199b
Summary:
Uglify@3.1.7 is causing some perf issues (more specifically, this commit: 5b4b07e9a7) that are impacting TTI on RN views and increased memory usage.
More specifically, code like:
```
function nonTrivialFn(pre) {
return pre + Math.random();
}
function hotFunctionCalledALotOfTimes(num) {
return nonTrivialFn(num + Math.random());
}
hotFunctionCalledALotOfTimes(3);
hotFunctionCalledALotOfTimes(5);
```
in v3.1.7 gets converted to:
```
function hotFunctionCalledALotOfTimes(num){
return function(pre) {
return pre + Math.random();
}(num + Math.random())
}
hotFunctionCalledALotOfTimes(3);
hotFunctionCalledALotOfTimes(5);
```
This causes a function creation each time `hotFunctionCalledALotOfTimes` is called.
By comparison, in v3.1.6, that previous code was converted to:
```
function nonTrivialFn(pre){
return pre + Math.random()
}
function hotFunctionCalledALotOfTimes(num){
return nonTrivialFn(num + Math.random())
}
hotFunctionCalledALotOfTimes(3);
hotFunctionCalledALotOfTimes(5);
```
Reviewed By: jeanlauliac, alexeylang
Differential Revision: D6296740
fbshipit-source-id: b3988d886e607103ec3ae6b9763b2f0411a8aa3c
Rafael Oleza
Peter van der Zee
Andres Suarez
Miguel Jimenez Esun
Christoph Nakazawa
Maël Nison
Jean Lauliac
Danny B
Bhuwan Khattar
James Ide
John Knox
Adam Ernst
glevi@fb.com
Ville Immonen
Charles Dick
Burak Yigit Kaya
Brian Shin
Nat Mote
David Aurelio
Adam Liechty
Marshall Roch
cpojer
Benjamin Weggersen
Caleb Meredith
