Skip to content

Add loc to each props in JSX4 #5937

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jan 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 38 additions & 14 deletions res_syntax/src/reactjs_jsx_v4.ml
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,9 @@ let recordFromProps ~loc ~removeKey callArguments =
(* let make = ({id, name, children}: props<'id, 'name, 'children>) *)
let makePropsTypeParamsTvar namedTypeList =
namedTypeList
|> List.filter_map (fun (_isOptional, label, _, _interiorType) ->
|> List.filter_map (fun (_isOptional, label, _, loc, _interiorType) ->
if label = "key" then None
else Some (Typ.var @@ safeTypeFromValue (Labelled label)))
else Some (Typ.var ~loc @@ safeTypeFromValue (Labelled label)))

let stripOption coreType =
match coreType with
Expand All @@ -268,7 +268,7 @@ let stripJsNullable coreType =
let makePropsTypeParams ?(stripExplicitOption = false)
?(stripExplicitJsNullableOfRef = false) namedTypeList =
namedTypeList
|> List.filter_map (fun (isOptional, label, _, interiorType) ->
|> List.filter_map (fun (isOptional, label, _, loc, interiorType) ->
if label = "key" then None
(* TODO: Worth thinking how about "ref_" or "_ref" usages *)
else if label = "ref" then
Expand All @@ -277,7 +277,7 @@ let makePropsTypeParams ?(stripExplicitOption = false)
For example, if JSX ppx is used for React Native, type would be different.
*)
match interiorType with
| {ptyp_desc = Ptyp_var "ref"} -> Some (refType Location.none)
| {ptyp_desc = Ptyp_var "ref"} -> Some (refType loc)
| _ ->
(* Strip explicit Js.Nullable.t in case of forwardRef *)
if stripExplicitJsNullableOfRef then stripJsNullable interiorType
Expand All @@ -287,9 +287,25 @@ let makePropsTypeParams ?(stripExplicitOption = false)
else if isOptional && stripExplicitOption then stripOption interiorType
else Some interiorType)

let makeLabelDecls ~loc namedTypeList =
let makeLabelDecls namedTypeList =
let rec checkDuplicatedLabel l =
let rec mem_label ((_, (la : string), _, _, _) as x) = function
| [] -> false
| (_, (lb : string), _, _, _) :: l -> lb = la || mem_label x l
in
match l with
| [] -> ()
| hd :: tl ->
if mem_label hd tl then
let _, label, _, loc, _ = hd in
React_jsx_common.raiseError ~loc "JSX: found the duplicated prop `%s`"
label
else checkDuplicatedLabel tl
in
let () = namedTypeList |> List.rev |> checkDuplicatedLabel in

namedTypeList
|> List.map (fun (isOptional, label, attrs, interiorType) ->
|> List.map (fun (isOptional, label, attrs, loc, interiorType) ->
if label = "key" then
Type.field ~loc ~attrs:(optionalAttrs @ attrs) {txt = label; loc}
interiorType
Expand All @@ -301,7 +317,7 @@ let makeLabelDecls ~loc namedTypeList =
(Typ.var @@ safeTypeFromValue @@ Labelled label))

let makeTypeDecls propsName loc namedTypeList =
let labelDeclList = makeLabelDecls ~loc namedTypeList in
let labelDeclList = makeLabelDecls namedTypeList in
(* 'id, 'className, ... *)
let params =
makePropsTypeParamsTvar namedTypeList
Expand Down Expand Up @@ -702,17 +718,22 @@ let argToType ~newtypes ~(typeConstraints : core_type option) types
in
match (type_, name, default) with
| Some type_, name, _ when isOptional name ->
(true, getLabel name, attrs, {type_ with ptyp_attributes = optionalAttrs})
( true,
getLabel name,
attrs,
loc,
{type_ with ptyp_attributes = optionalAttrs} )
:: types
| Some type_, name, _ -> (false, getLabel name, attrs, type_) :: types
| Some type_, name, _ -> (false, getLabel name, attrs, loc, type_) :: types
| None, name, _ when isOptional name ->
( true,
getLabel name,
attrs,
loc,
Typ.var ~loc ~attrs:optionalAttrs (safeTypeFromValue name) )
:: types
| None, name, _ when isLabelled name ->
(false, getLabel name, attrs, Typ.var ~loc (safeTypeFromValue name))
(false, getLabel name, attrs, loc, Typ.var ~loc (safeTypeFromValue name))
:: types
| _ -> types

Expand All @@ -721,10 +742,12 @@ let argWithDefaultValue (name, default, _, _, _, _) =
| Some default when isOptional name -> Some (getLabel name, default)
| _ -> None

let argToConcreteType types (name, attrs, _loc, type_) =
let argToConcreteType types (name, attrs, loc, type_) =
match name with
| name when isLabelled name -> (false, getLabel name, attrs, type_) :: types
| name when isOptional name -> (true, getLabel name, attrs, type_) :: types
| name when isLabelled name ->
(false, getLabel name, attrs, loc, type_) :: types
| name when isOptional name ->
(true, getLabel name, attrs, loc, type_) :: types
| _ -> types

let check_string_int_attribute_iter =
Expand Down Expand Up @@ -1306,7 +1329,8 @@ let transformSignatureItem ~config _mapper item =
makePropsRecordTypeSig ~coreTypeOfAttr ~typVarsOfCoreType "props"
psig_loc
((* If there is Nolabel arg, regard the type as ref in forwardRef *)
(if !hasForwardRef then [(true, "ref", [], refType Location.none)]
(if !hasForwardRef then
[(true, "ref", [], Location.none, refType Location.none)]
else [])
@ namedTypeList)
in
Expand Down
10 changes: 2 additions & 8 deletions res_syntax/tests/ppx/react/expected/aliasProps.res.txt
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
@@jsxConfig({version: 4, mode: "automatic"})

module C0 = {
type props<'priority, 'text> = {
priority: 'priority,
text?: 'text,
}
type props<'priority, 'text> = {priority: 'priority, text?: 'text}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder why this PR change leads to remove the trailing comma.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably formatted differently now because of the loc changes.


@react.component
let make = ({priority: _, ?text, _}: props<'priority, 'text>) => {
Expand All @@ -23,10 +20,7 @@ module C0 = {
}

module C1 = {
type props<'priority, 'text> = {
priority: 'priority,
text?: 'text,
}
type props<'priority, 'text> = {priority: 'priority, text?: 'text}

@react.component
let make = ({priority: p, ?text, _}: props<'priority, 'text>) => {
Expand Down
4 changes: 1 addition & 3 deletions res_syntax/tests/ppx/react/expected/commentAtTop.res.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
type props<'msg> = { // test React JSX file
msg: 'msg,
}
type props<'msg> = {msg: 'msg} // test React JSX file

@react.component
let make = ({msg, _}: props<'msg>) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@ let t = React.createElement(Foo.component, Foo.componentProps(~a=1, ~b={"1"}, ()
@@jsxConfig({version: 4, mode: "classic"})

module Foo = {
type props<'a, 'b> = {
a: 'a,
b: 'b,
}
type props<'a, 'b> = {a: 'a, b: 'b}

@module("Foo")
external component: React.componentLike<props<int, string>, React.element> = "component"
Expand All @@ -27,10 +24,7 @@ let t = React.createElement(Foo.component, {a: 1, b: "1"})
@@jsxConfig({version: 4, mode: "automatic"})

module Foo = {
type props<'a, 'b> = {
a: 'a,
b: 'b,
}
type props<'a, 'b> = {a: 'a, b: 'b}

@module("Foo")
external component: React.componentLike<props<int, string>, React.element> = "component"
Expand Down
8 changes: 2 additions & 6 deletions res_syntax/tests/ppx/react/expected/fileLevelConfig.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ module V3 = {
@@jsxConfig({version: 4, mode: "classic"})

module V4C = {
type props<'msg> = {
msg: 'msg,
}
type props<'msg> = {msg: 'msg}

@react.component
let make = ({msg, _}: props<'msg>) => {
Expand All @@ -36,9 +34,7 @@ module V4C = {
@@jsxConfig({version: 4, mode: "automatic"})

module V4A = {
type props<'msg> = {
msg: 'msg,
}
type props<'msg> = {msg: 'msg}

@react.component
let make = ({msg, _}: props<'msg>) => {
Expand Down
4 changes: 1 addition & 3 deletions res_syntax/tests/ppx/react/expected/interface.res.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
module A = {
type props<'x> = {
x: 'x,
}
type props<'x> = {x: 'x}
@react.component let make = ({x, _}: props<'x>) => React.string(x)
let make = {
let \"Interface$A" = (props: props<_>) => make(props)
Expand Down
4 changes: 1 addition & 3 deletions res_syntax/tests/ppx/react/expected/interface.resi.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
module A: {
type props<'x> = {
x: 'x,
}
type props<'x> = {x: 'x}
let make: React.componentLike<props<string>, React.element>
}

Expand Down
5 changes: 1 addition & 4 deletions res_syntax/tests/ppx/react/expected/interfaceWithRef.res.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
type props<'x, 'ref> = {
x: 'x,
ref?: 'ref,
}
type props<'x, 'ref> = {x: 'x, ref?: 'ref}
@react.component
let make = (
{x, _}: props<string, ReactDOM.Ref.currentDomRef>,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,2 @@
type props<'x, 'ref> = {
x: 'x,
ref?: 'ref,
}
type props<'x, 'ref> = {x: 'x, ref?: 'ref}
let make: React.componentLike<props<string, ReactDOM.Ref.currentDomRef>, React.element>
20 changes: 4 additions & 16 deletions res_syntax/tests/ppx/react/expected/mangleKeyword.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@ let c31 = React.createElement(C31.make, C31.makeProps(~_open="x", ()))
@@jsxConfig({version: 4, mode: "classic"})

module C4C0 = {
type props<'T_open, 'T_type> = {
@as("open") _open: 'T_open,
@as("type") _type: 'T_type,
}
type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type}

@react.component
let make = ({@as("open") _open, @as("type") _type, _}: props<'T_open, string>) =>
Expand All @@ -35,10 +32,7 @@ module C4C0 = {
}
}
module C4C1 = {
type props<'T_open, 'T_type> = {
@as("open") _open: 'T_open,
@as("type") _type: 'T_type,
}
type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type}

external make: @as("open") React.componentLike<props<string, string>, React.element> = "default"
}
Expand All @@ -49,10 +43,7 @@ let c4c1 = React.createElement(C4C1.make, {_open: "x", _type: "t"})
@@jsxConfig({version: 4, mode: "automatic"})

module C4A0 = {
type props<'T_open, 'T_type> = {
@as("open") _open: 'T_open,
@as("type") _type: 'T_type,
}
type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type}

@react.component
let make = ({@as("open") _open, @as("type") _type, _}: props<'T_open, string>) =>
Expand All @@ -64,10 +55,7 @@ module C4A0 = {
}
}
module C4A1 = {
type props<'T_open, 'T_type> = {
@as("open") _open: 'T_open,
@as("type") _type: 'T_type,
}
type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type}

external make: @as("open") React.componentLike<props<string, string>, React.element> = "default"
}
Expand Down
12 changes: 2 additions & 10 deletions res_syntax/tests/ppx/react/expected/newtype.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,7 @@ module V3 = {
@@jsxConfig({version: 4, mode: "classic"})

module V4C = {
type props<'a, 'b, 'c> = {
a: 'a,
b: 'b,
c: 'c,
}
type props<'a, 'b, 'c> = {a: 'a, b: 'b, c: 'c}

@react.component
let make = ({a, b, c, _}: props<'\"type-a", array<option<[#Foo('\"type-a")]>>, 'a>) =>
Expand All @@ -43,11 +39,7 @@ module V4C = {
@@jsxConfig({version: 4, mode: "automatic"})

module V4A = {
type props<'a, 'b, 'c> = {
a: 'a,
b: 'b,
c: 'c,
}
type props<'a, 'b, 'c> = {a: 'a, b: 'b, c: 'c}

@react.component
let make = ({a, b, c, _}: props<'\"type-a", array<option<[#Foo('\"type-a")]>>, 'a>) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ module User = {
type t = {firstName: string, lastName: string}

let format = user => "Dr." ++ user.lastName
type props<'doctor> = {
doctor: 'doctor,
}
type props<'doctor> = {doctor: 'doctor}

@react.component
let make = ({doctor, _}: props<'doctor>) => {
Expand Down
9 changes: 2 additions & 7 deletions res_syntax/tests/ppx/react/expected/removedKeyProp.res.txt
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
@@jsxConfig({version: 4, mode: "classic"})

module Foo = {
type props<'x, 'y> = {
x: 'x,
y: 'y,
}
type props<'x, 'y> = {x: 'x, y: 'y}

@react.component let make = ({x, y, _}: props<'x, 'y>) => React.string(x ++ y)
let make = {
Expand All @@ -15,9 +12,7 @@ module Foo = {
}

module HasChildren = {
type props<'children> = {
children: 'children,
}
type props<'children> = {children: 'children}

@react.component
let make = ({children, _}: props<'children>) => ReactDOM.createElement(React.fragment, [children])
Expand Down
10 changes: 2 additions & 8 deletions res_syntax/tests/ppx/react/expected/topLevel.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ module V3 = {
@@jsxConfig({version: 4, mode: "classic"})

module V4C = {
type props<'a, 'b> = {
a: 'a,
b: 'b,
}
type props<'a, 'b> = {a: 'a, b: 'b}

@react.component
let make = ({a, b, _}: props<'a, 'b>) => {
Expand All @@ -42,10 +39,7 @@ module V4C = {
@@jsxConfig({version: 4, mode: "automatic"})

module V4A = {
type props<'a, 'b> = {
a: 'a,
b: 'b,
}
type props<'a, 'b> = {a: 'a, b: 'b}

@react.component
let make = ({a, b, _}: props<'a, 'b>) => {
Expand Down
10 changes: 2 additions & 8 deletions res_syntax/tests/ppx/react/expected/typeConstraint.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ module V3 = {
@@jsxConfig({version: 4, mode: "classic"})

module V4C = {
type props<'a, 'b> = {
a: 'a,
b: 'b,
}
type props<'a, 'b> = {a: 'a, b: 'b}

@react.component
let make = ({a, b, _}: props<'\"type-a", '\"type-a">) =>
Expand All @@ -35,10 +32,7 @@ module V4C = {
@@jsxConfig({version: 4, mode: "automatic"})

module V4A = {
type props<'a, 'b> = {
a: 'a,
b: 'b,
}
type props<'a, 'b> = {a: 'a, b: 'b}

@react.component let make = ({a, b, _}: props<'\"type-a", '\"type-a">) => ReactDOM.jsx("div", {})
let make = {
Expand Down
Loading