Skip to content

Commit

Permalink
[flow][get-def] Better heuristics for def_loc of module ref of requir…
Browse files Browse the repository at this point in the history
…ed ESM module

Summary:
As discussed in D67869373, the current reason-based heuristic for module-ref logic is quite broken. This diff starts to use a new heuristic.

For ESM modules with default exports, we will use the default export as the def_loc of the require. Otherwise, we will pick the first export location instead. This is the same heuristic we use for the error location of common interface files: https://github.com/facebook/flow/blob/70811720be890cd85eb5dbf95fca2dbf960973bd/src/typing/module_info_analyzer.ml#L357-L389

Changelog: [internal]

Reviewed By: panagosg7

Differential Revision:
D67875259

------------------------------------------------------------------------
(from 18d2b3f9e08ff3d4182c0e9ec04754a62157ac93)

fbshipit-source-id: c07d84a494db0cd5514416c35d6de422658af57b
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Jan 7, 2025
1 parent 18717c9 commit 6443137
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 17 deletions.
62 changes: 46 additions & 16 deletions src/typing/flow_js_utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1558,26 +1558,56 @@ module CJSRequireTKit = struct
let types_tmap = Context.generate_property_map cx type_props in
NamespaceT { namespace_symbol = module_symbol; values_type; types_tmap }
in
if standard_cjs_esm_interop then
( lookup_builtin_typeapp
let t =
if standard_cjs_esm_interop then
lookup_builtin_typeapp
cx
reason
"$Flow$EsmModuleMarkerWrapperInModuleRef"
[mk_exports_namespace ()],
def_loc_of_reason reason
)
else
(* Use default export if option is enabled and module is not lib *)
let automatic_require_default =
(legacy_interop || Context.automatic_require_default cx)
&& not (is_lib_reason_def module_reason)
in
if automatic_require_default then
match NameUtils.Map.find_opt (OrdinaryName "default") value_exports_tmap with
| Some { preferred_def_locs = _; name_loc = _; type_ } -> (type_, def_loc_of_t type_)
| _ -> (mk_exports_namespace (), def_loc_of_reason reason)
[mk_exports_namespace ()]
else
(mk_exports_namespace (), def_loc_of_reason reason)
(* Use default export if option is enabled and module is not lib *)
let automatic_require_default =
(legacy_interop || Context.automatic_require_default cx)
&& not (is_lib_reason_def module_reason)
in
if automatic_require_default then
match NameUtils.Map.find_opt (OrdinaryName "default") value_exports_tmap with
| Some { preferred_def_locs = _; name_loc = _; type_ } -> type_
| _ -> mk_exports_namespace ()
else
mk_exports_namespace ()
in
let def_loc =
let def_loc_of_export { preferred_def_locs; name_loc; type_ } =
match preferred_def_locs with
| Some l -> Nel.hd l
| None ->
(match name_loc with
| Some l -> l
| None -> def_loc_of_t type_)
in
match NameUtils.Map.find_opt (OrdinaryName "default") value_exports_tmap with
| Some e -> def_loc_of_export e
| None ->
(match
NameUtils.Map.fold
(fun _ e acc ->
match acc with
| None -> Some (def_loc_of_export e)
| Some acc ->
let def_loc = def_loc_of_export e in
if ALoc.compare acc def_loc < 0 then
Some acc
else
Some def_loc)
value_exports_tmap
None
with
| Some l -> l
| None -> def_loc_of_reason module_reason)
in
(t, def_loc)
end

(* import * as X from 'SomeModule'; *)
Expand Down
3 changes: 3 additions & 0 deletions tests/get_def2/ParentESM2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// @flow

export const foo = 2;
6 changes: 5 additions & 1 deletion tests/get_def2/get_def2.exp
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,11 @@ Parent.js:4:18,4:28

module_ref.js:5:4
Flags:
module_ref.js:5:1,5:15
ParentESM.js:4:8,4:14

module_ref.js:7:4
Flags:
ParentESM2.js:3:14,3:16

refinements.js:10:4
Flags:
Expand Down
2 changes: 2 additions & 0 deletions tests/get_def2/module_ref.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@
// ^
'm#./ParentESM';
// ^
'm#./ParentESM2';
// ^

0 comments on commit 6443137

Please sign in to comment.