[flow] Make every module implicitly depend on react in file_sig

Summary:
## Background

I want to make all the React typing self-contained within the `react` module, instead of having the current setup where the stuff in the `react` module references things like `React$Node` from the global, so that we can override the typing of `React$Node`.

The self-contained goal is important so that we keep the ability to override react typing (by providing a `react.js.flow` in the userland in `flowtyped`), without relying on the current unprincipled global libdef overriding mechanism. Killing the current unprincipled global libdef overriding mechanism is important to implement declaration merging, which will be necessary for enhanced multiplatform support.

## Why this diff

In various places, we currently do refer to the global React types, especially `React$Node`. As a replacement, we should do something equivalent to `import * as React from 'react'. type T = React.Node` instead. In order to make this synthetic import work, it needs to be declared explicitly in file_sig.

Why do we need to unconditionally declare dependency on it? Well, reference to `React$Node` can happen at any file, due to subtyping rules that might grab a `React$Node` on the fly (e.g. function to component type subtyping). Fortunately, it only increases heap usage, but doesn't affect perf at all. For incremental recheck, it won't change the fanout compared to the status quo: no matter whether the user has a `react.js.flow` in the userland, changes to react typing will trigger a recheck on all files anyways, before and after.

Changelog: [internal]

Reviewed By: panagosg7

Differential Revision: D67991112

fbshipit-source-id: 0b176ea03e68a939fb570de2addb3d9e9104373c

src/parser_utils/__tests__/file_sig_test.ml

@@ -54,12 +54,20 @@ let assert_substrings_equal ~ctxt expected_remote expected_local source { remote
   assert_substring_equal ~ctxt expected_remote source remote_loc;
   assert_substring_equal ~ctxt expected_local source local_loc
 
+let requires_without_implicit_react_import file_sig =
+  file_sig
+  |> File_sig.requires
+  |> List.filter (function
+         | ImportSynthetic { source = "react" } -> false
+         | _ -> true
+         )
+
 let tests =
   "require"
   >::: [
          ( "cjs_require" >:: fun ctxt ->
            let source = "const Foo = require('foo')" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [
             Require
@@ -77,7 +85,7 @@ let tests =
          );
          ( "cjs_deep_requires" >:: fun ctxt ->
            let source = "let foo = {x: require('bar')}; func(foo, require('baz'));" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [
             Require
@@ -93,7 +101,7 @@ let tests =
          );
          ( "cjs_deep_requires_plus_bindings" >:: fun ctxt ->
            let source = "const Foo = require('foo'); func(Foo, require('bar'));" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [
             Require
@@ -115,7 +123,7 @@ let tests =
          );
          ( "cjs_require_template_literal" >:: fun ctxt ->
            let source = "const Foo = require(`foo`)" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [
             Require
@@ -133,7 +141,7 @@ let tests =
          );
          ( "cjs_require_named" >:: fun ctxt ->
            let source = "const {foo, bar: baz} = require('foo');" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [
             Require
@@ -165,7 +173,7 @@ let tests =
          );
          ( "cjs_require_duplicate_remote" >:: fun ctxt ->
            let source = "const {foo: bar, foo: baz} = require('foo');" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [
             Require
@@ -196,7 +204,7 @@ let tests =
          );
          ( "cjs_require_duplicate_local" >:: fun ctxt ->
            let source = "const {foo: bar, baz: bar} = require('foo');" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [
             Require
@@ -223,7 +231,7 @@ let tests =
            (* An initial version of the change to ban non-toplevel exports failed to descend into the RHS
             * of export statements *)
            let source = "module.exports.foo = require('foo');" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Require { source = (source_loc, "foo"); require_loc; bindings = None; prefix = _ }] ->
              assert_substring_equal ~ctxt "'foo'" source source_loc;
@@ -232,7 +240,7 @@ let tests =
          );
          ( "cjs_require_typeapp" >:: fun _ctxt ->
            let source = "const Foo = require<X>('foo')" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [] -> ()
            | _ -> assert_failure "Unexpected requires"
@@ -242,7 +250,7 @@ let tests =
            let parse_options =
              { Parser_env.default_parse_options with Parser_env.module_ref_prefix = Some "m#" }
            in
-           let requires = visit source ~parse_options |> File_sig.requires in
+           let requires = visit source ~parse_options |> requires_without_implicit_react_import in
            match requires with
            | [Require { source = (source_loc, "foo"); require_loc; bindings = _; prefix }] ->
              assert_substring_equal ~ctxt "'m#foo'" source source_loc;
@@ -254,7 +262,7 @@ let tests =
            let source = "graphql`query foo {}`" in
            let requires =
              visit source ~opts:{ default_opts with enable_relay_integration = true }
-             |> File_sig.requires
+             |> requires_without_implicit_react_import
            in
            match requires with
            | [Require { source = (source_loc, "foo.graphql"); require_loc; _ }] ->
@@ -273,7 +281,7 @@ let tests =
                    enable_relay_integration = true;
                    relay_integration_module_prefix = Some "./__generated__/";
                  }
-             |> File_sig.requires
+             |> requires_without_implicit_react_import
            in
            match requires with
            | [Require { source = (source_loc, "./__generated__/foo.graphql"); require_loc; _ }] ->
@@ -283,7 +291,7 @@ let tests =
          );
          ( "dynamic_import" >:: fun ctxt ->
            let source = "import('foo')" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [ImportDynamic { source = (source_loc, "foo"); import_loc }] ->
              assert_substring_equal ~ctxt "'foo'" source source_loc;
@@ -292,7 +300,7 @@ let tests =
          );
          ( "dynamic_import_template_literal" >:: fun ctxt ->
            let source = "import(`foo`)" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [ImportDynamic { source = (source_loc, "foo"); import_loc }] ->
              assert_substring_equal ~ctxt "`foo`" source source_loc;
@@ -301,14 +309,14 @@ let tests =
          );
          ( "es_import" >:: fun ctxt ->
            let source = "import 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import0 { source = (loc, "foo") }] -> assert_substring_equal ~ctxt "'foo'" source loc
            | _ -> assert_failure "Unexpected requires"
          );
          ( "es_import_default" >:: fun ctxt ->
            let source = "import Foo from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); named; _ }] ->
              named
@@ -320,7 +328,7 @@ let tests =
          );
          ( "es_import_named" >:: fun ctxt ->
            let source = "import {A} from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); named; _ }] ->
              named
@@ -332,7 +340,7 @@ let tests =
          );
          ( "es_import_renamed" >:: fun ctxt ->
            let source = "import {A as B} from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); named; _ }] ->
              named
@@ -344,7 +352,7 @@ let tests =
          );
          ( "es_import_named_type" >:: fun ctxt ->
            let source = "import {type A} from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); types; _ }] ->
              types
@@ -356,7 +364,7 @@ let tests =
          );
          ( "es_import_named_typeof" >:: fun ctxt ->
            let source = "import {typeof A} from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); typesof; _ }] ->
              typesof
@@ -368,15 +376,15 @@ let tests =
          );
          ( "es_import_ns" >:: fun ctxt ->
            let source = "import * as Foo from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); ns = Some (loc, "Foo"); _ }] ->
              assert_substring_equal ~ctxt "Foo" source loc
            | _ -> assert_failure "Unexpected requires"
          );
          ( "es_import_type" >:: fun ctxt ->
            let source = "import type A from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); types; _ }] ->
              types
@@ -388,7 +396,7 @@ let tests =
          );
          ( "es_import_type_named" >:: fun ctxt ->
            let source = "import type {A} from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); types; _ }] ->
              types
@@ -400,7 +408,7 @@ let tests =
          );
          ( "es_import_type_renamed" >:: fun ctxt ->
            let source = "import type {A as B} from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); types; _ }] ->
              types
@@ -412,7 +420,7 @@ let tests =
          );
          ( "es_import_typeof" >:: fun ctxt ->
            let source = "import typeof A from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); typesof; _ }] ->
              typesof
@@ -424,7 +432,7 @@ let tests =
          );
          ( "es_import_typeof_named" >:: fun ctxt ->
            let source = "import typeof {A} from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); typesof; _ }] ->
              typesof
@@ -436,7 +444,7 @@ let tests =
          );
          ( "es_import_typeof_renamed" >:: fun ctxt ->
            let source = "import typeof {A as B} from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); typesof; _ }] ->
              typesof
@@ -448,15 +456,15 @@ let tests =
          );
          ( "es_import_typesof_ns" >:: fun ctxt ->
            let source = "import typeof * as Foo from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); typesof_ns = Some (loc, "Foo"); _ }] ->
              assert_substring_equal ~ctxt "Foo" source loc
            | _ -> assert_failure "Unexpected requires"
          );
          ( "es_import_type_ns" >:: fun ctxt ->
            let source = "import type * as Foo from 'foo'" in
-           let requires = visit source |> File_sig.requires in
+           let requires = visit source |> requires_without_implicit_react_import in
            match requires with
            | [Import { source = (_, "foo"); ns; typesof_ns; _ }] ->
              assert_equal ~ctxt None ns;
@@ -466,7 +474,7 @@ let tests =
          ( "export_star" >:: fun ctxt ->
            let source = "export * from 'foo'" in
            let file_sig = visit source in
-           let requires = File_sig.requires file_sig in
+           let requires = requires_without_implicit_react_import file_sig in
            match requires with
            | [ExportFrom { source = (source_loc, "foo") }] ->
              assert_substring_equal ~ctxt "'foo'" source source_loc
@@ -475,7 +483,7 @@ let tests =
          ( "export_type_star" >:: fun ctxt ->
            let source = "export type * from 'foo'" in
            let file_sig = visit source in
-           let requires = File_sig.requires file_sig in
+           let requires = requires_without_implicit_react_import file_sig in
            match requires with
            | [ExportFrom { source = (source_loc, "foo") }] ->
              assert_substring_equal ~ctxt "'foo'" source source_loc
@@ -485,7 +493,7 @@ let tests =
            let source = "export * as ns from 'foo'" in
            let parse_options = Parser_env.default_parse_options in
            let file_sig = visit ~parse_options source in
-           let requires = File_sig.requires file_sig in
+           let requires = requires_without_implicit_react_import file_sig in
            match requires with
            | [ExportFrom { source = (source_loc, "foo") }] ->
              assert_substring_equal ~ctxt "'foo'" source source_loc
@@ -494,7 +502,7 @@ let tests =
          ( "declare_export_star" >:: fun ctxt ->
            let source = "declare export * from 'foo'" in
            let file_sig = visit source in
-           let requires = File_sig.requires file_sig in
+           let requires = requires_without_implicit_react_import file_sig in
            match requires with
            | [ExportFrom { source = (source_loc, "foo") }] ->
              assert_substring_equal ~ctxt "'foo'" source source_loc
@@ -504,7 +512,7 @@ let tests =
            let source = "declare export * as ns from 'foo'" in
            let parse_options = Parser_env.default_parse_options in
            let file_sig = visit ~parse_options source in
-           let requires = File_sig.requires file_sig in
+           let requires = requires_without_implicit_react_import file_sig in
            match requires with
            | [ExportFrom { source = (source_loc, "foo") }] ->
              assert_substring_equal ~ctxt "'foo'" source source_loc

src/parser_utils/file_sig.ml

@@ -167,7 +167,7 @@ class requires_calculator ~file_key ~ast ~opts =
 
     method private update_file_sig f = this#update_acc f
 
-    method private add_require require = this#update_file_sig (add_require require)
+    method add_require require = this#update_file_sig (add_require require)
 
     method private add_exports kind source =
       let open Ast.Statement in
@@ -231,12 +231,6 @@ class requires_calculator ~file_key ~ast ~opts =
       this#add_require (Require { source = (loc, mref); require_loc = loc; bindings = None; prefix });
       super#module_ref_literal loc lit
 
-    method! jsx_fragment loc expr =
-      (* Currently in statement.ml, we unconditionally use the React typing for jsx fragment without
-       * any customization ability. *)
-      this#add_require (ImportSynthetic { source = "react" });
-      super#jsx_fragment loc expr
-
     method! tagged_template loc (expr : ('loc, 'loc) Ast.Expression.TaggedTemplate.t) =
       let open Ast.Expression.TaggedTemplate in
       let { tag; quasi; comments = _ } = expr in
@@ -545,6 +539,7 @@ class requires_calculator ~file_key ~ast ~opts =
 
 let program ~file_key ~ast ~opts =
   let walk = new requires_calculator ~file_key ~ast ~opts in
+  walk#add_require (ImportSynthetic { source = "react" });
   (match file_key with
   | File_key.LibFile _ -> ()
   | _ -> walk#add_multiplatform_synthetic_imports);