add optional dependency support (#511)

Summary:
**Summary**

This PR added support for the common optional dependency pattern: enclose require() within try/catch block, which is already supported by webpack.

While the lack of optional dependency support is not new, due to [lean-core](facebook/react-native#23313) initiative, it has become obvious that we should just address this to provide better customer-experience. See the excellent discussion in react-native-community/discussions-and-proposals#120.

**Change Outline**
The changes are mainly in 3 areas:
1. visit try/catch block and mark optional dependencies (`collectDependencies.js`)
1. during dependency resolve, if the optional dependency fails to resolve, just ignore it. (`traverseDependencies.js`)
1. add a config (`optionalDependency`, defaults to true) to disable and customize (exclude dependency from being optional) (`metro-config`)

The rest are just tunneling through the new config option, added/modified tests.

**Test plan**

tested the new create react-native app with `react-native-vector-icons`  and optional `react-native-community/toolbar-android` (in try/catch block from `react-native-vector-icons` ) :
- without the optional dependency support, metro complained `react-native-community/toolbar-android` not found and abort
- with the optional dependency, the app starts up without any problem.

**Discussion**
- if we found `import()` in the try block, should we also consider it "optional"? The common pattern is to use `require()` but one would think `import()` should be just the same, no? This PR considered any dependency found in the try block "optional", but that can be changed easily.
- I put the new config `optionalDependency` in the config.resolver, not sure if it is best to be in resolver or transformer?
- should we add some kind of warning message for omitted optional dependency? verbose flag?
Pull Request resolved: #511

Reviewed By: rickhanlonii

Differential Revision: D20280664

Pulled By: cpojer

fbshipit-source-id: 8c4abebfbf2a2f4bfa65f64e2916c1e9182977f0

packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap

@@ -91,6 +91,7 @@ Object {
     "customizeFrame": [Function],
   },
   "transformer": Object {
+    "allowOptionalDependencies": false,
     "assetPlugins": Array [],
     "assetRegistryPath": "missing-asset-registry-path",
     "asyncRequireModulePath": "metro/src/lib/bundle-modules/asyncRequire",
@@ -224,6 +225,7 @@ Object {
     "customizeFrame": [Function],
   },
   "transformer": Object {
+    "allowOptionalDependencies": false,
     "assetPlugins": Array [],
     "assetRegistryPath": "missing-asset-registry-path",
     "asyncRequireModulePath": "metro/src/lib/bundle-modules/asyncRequire",

packages/metro-config/src/configTypes.flow.js

@@ -18,8 +18,8 @@ import type {
   DeltaResult,
   Graph,
   Module,
+  SerializerOptions,
 } from 'metro/src/DeltaBundler/types.flow.js';
-import type {SerializerOptions} from 'metro/src/DeltaBundler/types.flow';
 import type {TransformResult} from 'metro/src/DeltaBundler';
 import type {JsTransformerConfig} from 'metro/src/JSTransformer/worker';
 import type {TransformVariants} from 'metro/src/ModuleGraph/types.flow.js';

packages/metro-config/src/defaults/index.js

@@ -111,6 +111,7 @@ const getDefaultValues = (projectRoot: ?string): ConfigT => ({
     transformVariants: {default: {}},
     workerPath: 'metro/src/DeltaBundler/Worker',
     publicPath: '/assets',
+    allowOptionalDependencies: false,
   },
   cacheStores: [
     new FileStore({

packages/metro/src/DeltaBundler/__tests__/traverseDependencies-test.js

@@ -870,3 +870,82 @@ describe('reorderGraph', () => {
     ]);
   });
 });
+
+describe('optional dependencies', () => {
+  let localGraph;
+  let localOptions;
+  const getAllDependencies = () => {
+    const all = new Set();
+    mockedDependencyTree.forEach(deps => {
+      deps.forEach(r => all.add(r.name));
+    });
+    return all;
+  };
+  const assertResults = (dependencies, expectedMissing) => {
+    let count = 0;
+    const allDependency = getAllDependencies();
+    allDependency.forEach(m => {
+      const data = dependencies.get(`/${m}`);
+      if (expectedMissing.includes(m)) {
+        expect(data).toBeUndefined();
+      } else {
+        expect(data).not.toBeUndefined();
+      }
+      count += 1;
+    });
+    expect(count).toBeGreaterThan(0);
+    expect(count).toBe(allDependency.size);
+  };
+
+  const mockTransform = (notOptional?: string[]) => {
+    return function(path) {
+      const result = options.transform.apply(this, arguments);
+      result.dependencies.forEach(dep => {
+        if (notOptional && notOptional.includes(dep.name)) {
+          dep.data.isOptional = false;
+        } else {
+          dep.data.isOptional = dep.name.includes('optional-');
+        }
+      });
+      return result;
+    };
+  };
+
+  beforeEach(() => {
+    mockedDependencies = new Set();
+    mockedDependencyTree = new Map();
+
+    entryModule = Actions.createFile('/bundle-o');
+
+    Actions.addDependency('/bundle-o', '/regular-a');
+    Actions.addDependency('/bundle-o', '/optional-b');
+
+    Actions.deleteFile('/optional-b');
+
+    localGraph = {
+      dependencies: new Map(),
+      entryPoints: ['/bundle-o'],
+    };
+  });
+
+  it('missing optional dependency will be skipped', async () => {
+    localOptions = {
+      ...options,
+      transform: mockTransform(),
+    };
+
+    const result = await initialTraverseDependencies(localGraph, localOptions);
+
+    const dependencies = result.added;
+    assertResults(dependencies, ['optional-b']);
+  });
+  it('missing non-optional dependency will throw', async () => {
+    localOptions = {
+      ...options,
+      transform: mockTransform(['optional-b']),
+    };
+    await expect(
+      initialTraverseDependencies(localGraph, localOptions),
+    ).rejects.toThrow();
+  });
+});

packages/metro/src/DeltaBundler/traverseDependencies.js

@@ -471,18 +471,37 @@ function resolveDependencies<T>(
   dependencies: $ReadOnlyArray<TransformResultDependency>,
   options: InternalOptions<T>,
 ): Map<string, Dependency> {
-  return new Map(
-    dependencies.map((result: TransformResultDependency) => {
-      const relativePath = result.name;
-
-      const dependency = {
-        absolutePath: options.resolve(parentPath, result.name),
-        data: result,
-      };
+  const resolve = (parentPath: string, result: TransformResultDependency) => {
+    const relativePath = result.name;
+    try {
+      return [
+        relativePath,
+        {
+          absolutePath: options.resolve(parentPath, relativePath),
+          data: result,
+        },
+      ];
+    } catch (error) {
+      // Ignore unavailable optional dependencies. They are guarded
+      // with a try-catch block and will be handled during runtime.
+      if (result.data.isOptional !== true) {
+        throw error;
+      }
+    }
+    return undefined;
+  };
 
-      return [relativePath, dependency];
-    }),
+  const resolved = dependencies.reduce(
+    (list: Array<[string, Dependency]>, result: TransformResultDependency) => {
+      const resolvedPath = resolve(parentPath, result);
+      if (resolvedPath) {
+        list.push(resolvedPath);
+      }
+      return list;
+    },
+    [],
   );
+  return new Map(resolved);
 }
 
 /**

packages/metro/src/DeltaBundler/types.flow.js

@@ -42,6 +42,10 @@ export type TransformResultDependency = {|
     +splitCondition?: {|
       +mobileConfigName: string,
     |},
+    /**
+     * The dependency is enclosed in a try/catch block.
+     */
+    +isOptional?: boolean,
 
     +locs: $ReadOnlyArray<BabelSourceLocation>,
   |},
@@ -79,6 +83,12 @@ export type TransformResultWithSource<T = MixedOutput> = $ReadOnly<{|
 export type TransformFn<T = MixedOutput> = string => Promise<
   TransformResultWithSource<T>,
 >;
+export type AllowOptionalDependenciesWithOptions = {|
+  +exclude: Array<string>,
+|};
+export type AllowOptionalDependencies =
+  | boolean
+  | AllowOptionalDependenciesWithOptions;
 
 export type Options<T = MixedOutput> = {|
   +resolve: (from: string, to: string) => string,

packages/metro/src/JSTransformer/worker.js

@@ -35,6 +35,7 @@ const {
   toSegmentTuple,
 } = require('metro-source-map');
 import type {TransformResultDependency} from 'metro/src/DeltaBundler';
+import type {AllowOptionalDependencies} from 'metro/src/DeltaBundler/types.flow.js';
 import type {DynamicRequiresBehavior} from '../ModuleGraph/worker/collectDependencies';
 import type {
   BasicSourceMap,
@@ -68,6 +69,7 @@ export type JsTransformerConfig = $ReadOnly<{|
   minifierPath: string,
   optimizationSizeLimit: number,
   publicPath: string,
+  allowOptionalDependencies: AllowOptionalDependencies,
 |}>;
 
 import type {CustomTransformOptions} from 'metro-babel-transformer';
@@ -280,6 +282,7 @@ class JsTransformer {
           ),
           inlineableCalls: [importDefault, importAll],
           keepRequireNames: options.dev,
+          allowOptionalDependencies: this._config.allowOptionalDependencies,
         };
         ({ast, dependencies, dependencyMapName} = collectDependencies(
           ast,