Skip to content

Commit

Permalink
module.system.node.allow_root_relative
Browse files Browse the repository at this point in the history
Summary:
#8156 made me aware that some users want to be able to import their code using root-relative paths. They were misusing `module.system.node.resolve_dirname`, which worked for them due to a bug, and experiencing odd behaviors. This stack of diffs demonstrates the behavior, fixes the bug (breaking the use case), and adds first-class support for this use case.

This diff adds a new `.flowconfig` option which allows you to import modules using a root-relative path.

Reviewed By: jbrown215

Differential Revision: D18190689

fbshipit-source-id: e61325ee847361a2873327b8302f3662799d6f58
  • Loading branch information
gabelevi authored and facebook-github-bot committed Oct 30, 2019
1 parent 11d5b89 commit c6a56c6
Show file tree
Hide file tree
Showing 17 changed files with 358 additions and 1 deletion.
2 changes: 2 additions & 0 deletions src/commands/commandUtils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1280,6 +1280,8 @@ let make_options ~flowconfig_name ~flowconfig ~lazy_mode ~root (options_flags :
opt_saved_state_force_recheck = options_flags.saved_state_force_recheck;
opt_saved_state_no_fallback = options_flags.saved_state_no_fallback;
opt_no_saved_state = options_flags.no_saved_state;
opt_node_resolver_allow_root_relative =
FlowConfig.node_resolver_allow_root_relative flowconfig;
opt_arch;
opt_abstract_locations;
opt_cache_direct_dependents = FlowConfig.cache_direct_dependents flowconfig;
Expand Down
6 changes: 6 additions & 0 deletions src/commands/config/flowConfig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ module Opts = struct
modules_are_use_strict: bool;
munge_underscores: bool;
no_flowlib: bool;
node_resolver_allow_root_relative: bool;
node_resolver_dirnames: string list;
recursion_limit: int;
root_name: string option;
Expand Down Expand Up @@ -188,6 +189,7 @@ module Opts = struct
modules_are_use_strict = false;
munge_underscores = false;
no_flowlib = false;
node_resolver_allow_root_relative = false;
node_resolver_dirnames = ["node_modules"];
recursion_limit = 10000;
root_name = None;
Expand Down Expand Up @@ -453,6 +455,8 @@ module Opts = struct
( "module.system",
enum [("node", Options.Node); ("haste", Options.Haste)] (fun opts v ->
Ok { opts with module_system = v }) );
( "module.system.node.allow_root_relative",
boolean (fun opts v -> Ok { opts with node_resolver_allow_root_relative = v }) );
( "module.system.node.resolve_dirname",
string
~init:(fun opts -> { opts with node_resolver_dirnames = [] })
Expand Down Expand Up @@ -1175,6 +1179,8 @@ let munge_underscores c = c.options.Opts.munge_underscores

let no_flowlib c = c.options.Opts.no_flowlib

let node_resolver_allow_root_relative c = c.options.Opts.node_resolver_allow_root_relative

let node_resolver_dirnames c = c.options.Opts.node_resolver_dirnames

let recursion_limit c = c.options.Opts.recursion_limit
Expand Down
2 changes: 2 additions & 0 deletions src/commands/config/flowConfig.mli
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ val munge_underscores : config -> bool

val no_flowlib : config -> bool

val node_resolver_allow_root_relative : config -> bool

val node_resolver_dirnames : config -> string list

val required_version : config -> string option
Expand Down
3 changes: 3 additions & 0 deletions src/common/options.ml
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ type t = {
opt_module_name_mappers: (Str.regexp * string) list;
opt_modules_are_use_strict: bool;
opt_munge_underscores: bool;
opt_node_resolver_allow_root_relative: bool;
opt_no_saved_state: bool;
opt_profile: bool;
opt_quiet: bool;
Expand Down Expand Up @@ -200,6 +201,8 @@ let modules_are_use_strict opts = opts.opt_modules_are_use_strict

let no_saved_state opts = opts.opt_no_saved_state

let node_resolver_allow_root_relative opts = opts.opt_node_resolver_allow_root_relative

let recursion_limit opts = opts.opt_recursion_limit

let root opts = opts.opt_root
Expand Down
20 changes: 19 additions & 1 deletion src/services/inference/module/module_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -362,10 +362,28 @@ module Node = struct
let resolve_import ~options ~reader node_modules_containers f loc ?resolution_acc import_str =
let file = File_key.to_string f in
let dir = Filename.dirname file in
let root_str = Options.root options |> Path.to_string in
if explicitly_relative import_str || absolute import_str then
resolve_relative ~options ~reader loc ?resolution_acc dir import_str
else
node_module ~options ~reader node_modules_containers f loc resolution_acc dir import_str
lazy_seq
[
lazy
( if Options.node_resolver_allow_root_relative options then
resolve_relative ~options ~reader loc ?resolution_acc root_str import_str
else
None );
lazy
(node_module
~options
~reader
node_modules_containers
f
loc
resolution_acc
dir
import_str);
]

let imported_module ~options ~reader node_modules_containers file loc ?resolution_acc import_str
=
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[options]
module.system.node.allow_root_relative=true
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// @flow

const value: "user_code" = "user_code";
export default value;
Original file line number Diff line number Diff line change
@@ -0,0 +1,238 @@
Error ----------------------------------------------------------------------------------------------- subdir/test.js:7:2

Cannot cast `ambiguous` to empty because string literal `user_code` [1] is incompatible with empty [2].

subdir/test.js:7:2
7| (ambiguous: empty)
^^^^^^^^^

References:
ambiguous.js:3:14
3| const value: "user_code" = "user_code";
^^^^^^^^^^^ [1]
subdir/test.js:7:13
7| (ambiguous: empty)
^^^^^ [2]


Error ---------------------------------------------------------------------------------------------- subdir/test.js:10:2

Cannot cast `sub_ambiguous` to empty because string literal `user_code` [1] is incompatible with empty [2].

subdir/test.js:10:2
10| (sub_ambiguous: empty)
^^^^^^^^^^^^^

References:
subdir/ambiguous.js:3:14
3| const value: "user_code" = "user_code";
^^^^^^^^^^^ [1]
subdir/test.js:10:17
10| (sub_ambiguous: empty)
^^^^^ [2]


Error ---------------------------------------------------------------------------------------------- subdir/test.js:14:2

Cannot cast `user_code` to empty because string literal `user_code` [1] is incompatible with empty [2].

subdir/test.js:14:2
14| (user_code: empty)
^^^^^^^^^

References:
user_code.js:3:14
3| const value: "user_code" = "user_code";
^^^^^^^^^^^ [1]
subdir/test.js:14:13
14| (user_code: empty)
^^^^^ [2]


Error ---------------------------------------------------------------------------------------------- subdir/test.js:17:2

Cannot cast `sub_user_code` to empty because string literal `user_code` [1] is incompatible with empty [2].

subdir/test.js:17:2
17| (sub_user_code: empty)
^^^^^^^^^^^^^

References:
subdir/user_code.js:3:14
3| const value: "user_code" = "user_code";
^^^^^^^^^^^ [1]
subdir/test.js:17:17
17| (sub_user_code: empty)
^^^^^ [2]


Error ---------------------------------------------------------------------------------------------- subdir/test.js:21:2

Cannot cast `node_code` to empty because string literal `node_code` [1] is incompatible with empty [2].

subdir/test.js:21:2
21| (node_code: empty)
^^^^^^^^^

References:
node_modules/node_code.js:3:14
3| const value: "node_code" = "node_code";
^^^^^^^^^^^ [1]
subdir/test.js:21:13
21| (node_code: empty)
^^^^^ [2]


Error ---------------------------------------------------------------------------------------------- subdir/test.js:24:2

Cannot cast `sub_node_code` to empty because string literal `node_code` [1] is incompatible with empty [2].

subdir/test.js:24:2
24| (sub_node_code: empty)
^^^^^^^^^^^^^

References:
node_modules/subdir/node_code.js:3:14
3| const value: "node_code" = "node_code";
^^^^^^^^^^^ [1]
subdir/test.js:24:17
24| (sub_node_code: empty)
^^^^^ [2]


Error --------------------------------------------------------------------------------------------- subdir/test.js:27:25

Cannot resolve module `nonexistent`.

27| import nonexistent from 'nonexistent'
^^^^^^^^^^^^^


Error --------------------------------------------------------------------------------------------- subdir/test.js:28:29

Cannot resolve module `subdir/nonexistent`.

28| import sub_nonexistent from 'subdir/nonexistent'
^^^^^^^^^^^^^^^^^^^^


Error ------------------------------------------------------------------------------------------------------ test.js:5:2

Cannot cast `ambiguous` to empty because string literal `user_code` [1] is incompatible with empty [2].

test.js:5:2
5| (ambiguous: empty)
^^^^^^^^^

References:
ambiguous.js:3:14
3| const value: "user_code" = "user_code";
^^^^^^^^^^^ [1]
test.js:5:13
5| (ambiguous: empty)
^^^^^ [2]


Error ------------------------------------------------------------------------------------------------------ test.js:8:2

Cannot cast `sub_ambiguous` to empty because string literal `user_code` [1] is incompatible with empty [2].

test.js:8:2
8| (sub_ambiguous: empty)
^^^^^^^^^^^^^

References:
subdir/ambiguous.js:3:14
3| const value: "user_code" = "user_code";
^^^^^^^^^^^ [1]
test.js:8:17
8| (sub_ambiguous: empty)
^^^^^ [2]


Error ----------------------------------------------------------------------------------------------------- test.js:12:2

Cannot cast `user_code` to empty because string literal `user_code` [1] is incompatible with empty [2].

test.js:12:2
12| (user_code: empty)
^^^^^^^^^

References:
user_code.js:3:14
3| const value: "user_code" = "user_code";
^^^^^^^^^^^ [1]
test.js:12:13
12| (user_code: empty)
^^^^^ [2]


Error ----------------------------------------------------------------------------------------------------- test.js:15:2

Cannot cast `sub_user_code` to empty because string literal `user_code` [1] is incompatible with empty [2].

test.js:15:2
15| (sub_user_code: empty)
^^^^^^^^^^^^^

References:
subdir/user_code.js:3:14
3| const value: "user_code" = "user_code";
^^^^^^^^^^^ [1]
test.js:15:17
15| (sub_user_code: empty)
^^^^^ [2]


Error ----------------------------------------------------------------------------------------------------- test.js:19:2

Cannot cast `node_code` to empty because string literal `node_code` [1] is incompatible with empty [2].

test.js:19:2
19| (node_code: empty)
^^^^^^^^^

References:
node_modules/node_code.js:3:14
3| const value: "node_code" = "node_code";
^^^^^^^^^^^ [1]
test.js:19:13
19| (node_code: empty)
^^^^^ [2]


Error ----------------------------------------------------------------------------------------------------- test.js:22:2

Cannot cast `sub_node_code` to empty because string literal `node_code` [1] is incompatible with empty [2].

test.js:22:2
22| (sub_node_code: empty)
^^^^^^^^^^^^^

References:
node_modules/subdir/node_code.js:3:14
3| const value: "node_code" = "node_code";
^^^^^^^^^^^ [1]
test.js:22:17
22| (sub_node_code: empty)
^^^^^ [2]


Error ---------------------------------------------------------------------------------------------------- test.js:25:25

Cannot resolve module `nonexistent`.

25| import nonexistent from 'nonexistent'
^^^^^^^^^^^^^


Error ---------------------------------------------------------------------------------------------------- test.js:26:29

Cannot resolve module `subdir/nonexistent`.

26| import sub_nonexistent from 'subdir/nonexistent'
^^^^^^^^^^^^^^^^^^^^



Found 16 errors

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// @flow

const value: "user_code" = "user_code";
export default value;
28 changes: 28 additions & 0 deletions tests/config_module_system_node_allow_root_relative/subdir/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// @flow
// Same file as test.js in the root directory.
// This tests that the paths really are root-relative

// These exist in both user code and node_modules, but user code wins
import ambiguous from 'ambiguous';
(ambiguous: empty)

import sub_ambiguous from 'subdir/ambiguous';
(sub_ambiguous: empty)

// These exist in only user code
import user_code from 'user_code';
(user_code: empty)

import sub_user_code from 'subdir/user_code';
(sub_user_code: empty)

// These exist in only node code
import node_code from 'node_code';
(node_code: empty)

import sub_node_code from 'subdir/node_code';
(sub_node_code: empty)

// These exist nowhere
import nonexistent from 'nonexistent'
import sub_nonexistent from 'subdir/nonexistent'
Loading

0 comments on commit c6a56c6

Please sign in to comment.